Page MenuHomePhabricator

[dagster-aws] Make dagster_aws imports more sane
ClosedPublic

Authored by nate on Thu, Mar 26, 12:31 AM.

Details

Summary

This was really starting to bother me so I took a pass at a more sane import organization for dagster-aws. We were previously inconsistently exporting some things at dagster_aws root, some things at e.g. dagster_aws.s3, and some you'd have to reach all the way down into e.g. dagster_aws.s3.utils.

Now, public APIs are exported at:

dagster_aws.cloudwatch
dagster_aws.emr
dagster_aws.redshift
dagster_aws.s3

This felt better to me than exporting everything at dagster_aws root, because in the limit there will be a large number of AWS services we'll eventually support.

Test Plan

unit

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

nate created this revision.Thu, Mar 26, 12:31 AM
nate edited the summary of this revision. (Show Details)Thu, Mar 26, 12:37 AM
nate updated this revision to Diff 11039.Fri, Mar 27, 3:52 PM
nate edited the summary of this revision. (Show Details)

update autodocs links

is any of this going to be a breaking change for users?

nate added a comment.Fri, Mar 27, 5:47 PM

depending on where they're doing their imports from, it could be, so would want to document / add to changelog / post in #fresh-on-master etc.

max added a comment.Fri, Mar 27, 5:55 PM

this is probably long term right though the examples mostly feel like usability hits. can you write the changelog entry before merging?

docs/sections/api/apidocs/libraries/dagster_aws.rst
66–67

maybe group these with the appropriate service

max accepted this revision.Fri, Mar 27, 5:55 PM
This revision is now accepted and ready to land.Fri, Mar 27, 5:55 PM
nate updated this revision to Diff 11053.Fri, Mar 27, 9:18 PM

comments

This revision was automatically updated to reflect the committed changes.

Is there a way we can do this with a deprecation strategy so ease transition?

nate added a comment.Sat, Mar 28, 2:05 PM

yeah saw the github issue, will add back some of the imports until we release again. should be able to make both import from root and import from sub module work. didn’t consider the mixed git clone / PyPI case