Re: svn commit: r1869982 - in /subversion/trunk/tools/dist: release-lines.yaml release.py

[Daniel Shahaf <d....@daniel.shahaf.name>]

julianfoad@apache.org wrote on Mon, 18 Nov 2019 17:00 +00:00:
> +++ subversion/trunk/tools/dist/release.py Mon Nov 18 17:00:16 2019
> @@ -70,43 +71,22 @@ except ImportError:
> +# Read the dist metadata (about release lines)
> +with open(get_dist_metadata_file_path(), 'r') as stream:
> +    dist_metadata = yaml.load(stream)

yaml.load() is/was unsafe:

https://github.com/yaml/pyyaml/wiki/PyYAML-yaml.load%28input%29-Deprecation

yaml.safe_load() should be used instead.

----

Separately, at the risk of bikeshedding, I'd suggest to use json, for
two reasons:

- It's part of the Python stdlib.

- jq(1) exists.

(Yes, I'm happy to make the change myself if needed.)

Cheers,

Daniel

Re: svn commit: r1869982 - in /subversion/trunk/tools/dist: release-lines.yaml release.py

[Julian Foad <ju...@foad.me.uk>]

Daniel Shahaf wrote:
> julianfoad@apache.org wrote on Mon, 18 Nov 2019 17:00 +00:00:
>> +++ subversion/trunk/tools/dist/release.py Mon Nov 18 17:00:16 2019
>> @@ -70,43 +71,22 @@ except ImportError:
>> +# Read the dist metadata (about release lines)
>> +with open(get_dist_metadata_file_path(), 'r') as stream:
>> +    dist_metadata = yaml.load(stream)
> 
> yaml.load() is/was unsafe:
> 
> https://github.com/yaml/pyyaml/wiki/PyYAML-yaml.load%28input%29-Deprecation
> 
> yaml.safe_load() should be used instead.

Meh, ok, didn't know that; will change it.

> Separately, at the risk of bikeshedding, I'd suggest to use json, for
> two reasons:
> 
> - It's part of the Python stdlib.
> 
> - jq(1) exists.

But this isn't machine communication, it's human input and so needs the 
ability to have at least comments and preferably other conveniences (I 
used define-and-reference, for example).

- Julian