Page MenuHomePhabricator

use backwards-compatible yaml api

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



We added a dependency on yaml.FullLoader in D2287, which was only introduced in pyyaml 5.1 (see

Test Plan

Ran make dev_install, saw tests pass

Diff Detail

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

Event Timeline

prha created this revision.Mar 20 2020, 8:39 PM
prha added a comment.EditedMar 20 2020, 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.Mar 20 2020, 8:46 PM

q mgmt to make sure @prha sees this

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

we can call yaml.load without the Loader arg, and it will generate a warning:

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.Mar 20 2020, 8:55 PM

update callsite instead of pinning

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

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

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