Visidata: pyaml library updated, loader needs Loader= parameter

[frosencrantz]

The pyyaml library has changed since the yaml loader was last updated. There is a new requirement for adding Loader= parameter.

/usr/lib/python3.6/site-packages/visidata/loaders/yaml.py:13: YAMLLoadWarning: calling yaml.load() without Loader=... is deprecated, as the default Loader is unsafe. Please read https://msg.pyyaml.org/load for full details.

[anjakefala]

Hi @frosencrantz, thanks for letting me know about this!

For now, I am going to set an upper limit on the version of the pyaml library that we support. (I should have been doing the same for all of our dependencies.)

I have also added looking into this more deeply, and possibly modifying the yaml loader code into my queue. If anyone wants to take the reigns on this investigation (which minimum versions of pyaml supports the Loader= parameter being the main question), they are welcome to.

[anjakefala]

I ended up making the code change and bumping the minimum PyYAML version!

[tsibley]

I'm looking at the YAML loader because I'd like to add support for YAML files with multiple documents, and I'm wondering why the full loader is used instead of the safe loader? Ideally, I'd like to be able to trust vd to open files from unknown sources without verifying them first another way.

[anjakefala]

There was not a thoughtful reason behind that decision. If you would like to open a PR with a modification to use the safe loader, that would be welcome!

[tsibley]

@anjakefala Great! I submitted #600. Thanks for your quick reply!

[anjakefala]

@tsibley If someone requests the full loader, or if @saulpw things it would be a good idea, we can negotiate adding an --unsafe option. But the default should be safe because VisiData is often used as a first-pass tool. Thanks for writing up the change so quickly!

[saulpw]

Of course it should be yaml_unsafe, unless we wanted to combine it with safety_first somehow (but that's False/unsafe by default, and I agree, this should be safe by default as it's more of a security rather than data issue).

[anjakefala]

I do not think we want to combine it with safety_first for the reasons you outlined.

My only hesitation with yaml_unsafe was knowing that you have a hesitance with option-deluge. If it is one that you would like to have, I can add it!

[saulpw]

Let's not add it proactively, but if someone desires the unsafe behavior, then we can add it.