Opened 8 years ago

Closed 7 years ago

#5268 closed (fixed)

Remove Python specific types from YAML output (use safe_dump?)

Reported by: django@… Owned by: poelzi
Component: Core (Serialization) Version: master
Severity: Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

dumps produced by yaml.dump are not very language neutral.
safe_dump=True as serialize parameter will call yaml.safe_dump which doesn't have this problem.

Attachments (2)

patch_yaml_safe_dumper.txt (1.6 KB) - added by django@… 8 years ago.
add safe_dump argument to yaml serializer
yaml_patch.diff (1.6 KB) - added by poelzi 7 years ago.
better patch. use safe_dump with tests

Download all attachments as: .zip

Change History (8)

Changed 8 years ago by django@…

add safe_dump argument to yaml serializer

comment:1 Changed 8 years ago by russellm

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

Configurability is great, but is there any compelling reason that we shouldn't just change the implementation to use safe_dump all the time? I'm not a YAML expert, but looking at the PyYAML docs, it appears that the only place the difference would matter is when serializing 'arbitrary Python objects'. I don't think we need this capability, and if we are, we should be able to work around it without too much difficulty.

comment:2 Changed 8 years ago by django@…

a unicode object for example is considered a 'arbitrary Python objects'. this creates such nice looking dumps like:

https://db.leipzig.freifunk.net/dns/yaml/domain/list/

the !!python/unicode is not very portable. however, i asked the yaml programmer about this issue befor filling this bug
and he said suggested using safe_dump.
i'm not so sure about this, using dump has many advantages when it comes to terms of mapping verify presice things.
I'm thinking about writing a extended mapper that causes relations to be mapped like the model.
But for portability reasons this is not suggested, therefor I suggest this argument.

comment:3 Changed 8 years ago by anonymous

  • Triage Stage changed from Unreviewed to Design decision needed

comment:4 Changed 7 years ago by russellm

  • Needs tests set
  • Patch needs improvement set
  • Summary changed from add yaml save_dump argument to Remove Python specific types from YAML output (use safe_dump?)
  • Triage Stage changed from Design decision needed to Accepted

Ok. I'm -1 to adding a safe_dump argument to the serializer. However, I agree that the output format is not pretty. I've changed the ticket title to reflect the underlying problem.

It looks as if the YAML output has changed significantly since the unicode branch was merged - this is the underlying problem that should be solved. The solution may be to switch to using safe_dump all the time - I'll leave that decision to whoever tackles this problem.

Changed 7 years ago by poelzi

better patch. use safe_dump with tests

comment:5 Changed 7 years ago by poelzi

  • Needs tests unset
  • Owner changed from nobody to poelzi
  • Patch needs improvement unset
  • Status changed from new to assigned
  • Triage Stage changed from Accepted to Ready for checkin

comment:6 Changed 7 years ago by jacob

  • Resolution set to fixed
  • Status changed from assigned to closed

(In [6891]) Fixed #5268: the YAML serializer now uses yaml.safe_dump() instead the plain yaml.dump(); this makes YAML dumps more portable, and also removes the crufty '!!python/unicode' business. Thanks, poelzi.

Note: See TracTickets for help on using tickets.
Back to Top