Page MenuHomePhabricator

use backwards-compatible yaml api
ClosedPublic

Authored by prha on Fri, Mar 20, 8:39 PM.

Details

Summary

We added a dependency on yaml.FullLoader in D2287, which was only introduced in pyyaml 5.1 (see https://pyyaml.org/wiki/PyYAML#history)

Test Plan

Ran make dev_install, saw tests pass

Diff Detail

Repository
R1 dagster
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

prha created this revision.Fri, Mar 20, 8:39 PM
prha added a comment.EditedFri, Mar 20, 8:40 PM

this was previously failing for me locally, because i had PyYAML==3.13 installed

do we really need the callsite to yaml.FullLoader tho?

can we fix this by using a backwards compatible api? i want to be pretty sensitive to strict deps

ya what alex said

schrockn requested changes to this revision.Fri, Mar 20, 8:46 PM

q mgmt to make sure @prha sees this

This revision now requires changes to proceed.Fri, Mar 20, 8:46 PM
prha added a comment.Fri, Mar 20, 8:48 PM

we can call yaml.load without the Loader arg, and it will generate a warning:
https://github.com/yaml/pyyaml/wiki/PyYAML-yaml.load(input)-Deprecation

We can suppress the warning using env vars also, I suppose

-            telemetry_id_yaml = yaml.load(telemetry_id_file, Loader=yaml.FullLoader)
+            telemetry_id_yaml = yaml.safe_load(telemetry_id_file)
prha updated this revision to Diff 10767.Fri, Mar 20, 8:55 PM

update callsite instead of pinning

prha retitled this revision from add pyyaml constraint to use backwards-compatible yaml api.Fri, Mar 20, 8:56 PM
max added a comment.Fri, Mar 20, 8:57 PM

it might be nice to centralize our invocations of yaml.load in dagster utils somewhere

max accepted this revision.Fri, Mar 20, 8:57 PM
schrockn accepted this revision.Fri, Mar 20, 8:59 PM
This revision is now accepted and ready to land.Fri, Mar 20, 8:59 PM
This revision was landed with ongoing or failed builds.Fri, Mar 20, 8:59 PM
This revision was automatically updated to reflect the committed changes.