Page MenuHomePhabricator

Clean up up S3 resource (#2292)
ClosedPublic

Authored by nate on Sat, Mar 21, 5:41 PM.

Details

Summary

Per #2292, resources like S3 shouldn't attempt to mirror underlying APIs, otherwise we'll always be playing catch-up with those of the underlying 3rd party library (not to mention handling multiple version compatibility).

This diff drops that behavior, and users will now need to access the session property on the S3 resource to access the underlying boto3 S3 resource.

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.Sat, Mar 21, 5:41 PM
nate edited the summary of this revision. (Show Details)Sat, Mar 21, 6:05 PM
nate added reviewers: max, schrockn.
schrockn requested changes to this revision.Sat, Mar 21, 7:22 PM

How about we just eliminate S3Resource class totally and just make the s3 property a boto client?

This revision now requires changes to proceed.Sat, Mar 21, 7:22 PM
nate added a comment.Sat, Mar 21, 8:02 PM

yeah agree that's better - updated to remove S3Resource throughout

This revision is now accepted and ready to land.Sat, Mar 21, 8:29 PM
This revision was automatically updated to reflect the committed changes.