Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add transfer operator S3 to (generic) SQL #28964
Add transfer operator S3 to (generic) SQL #28964
Changes from 11 commits
1fd240f
b84cc56
cd204a3
a1394dc
02c3252
172e6e9
f147113
74a63ec
078ec76
d13cd79
e313bb7
f6893a1
197dd54
49c921a
28f0cd5
feab443
24cc4d5
07af1b7
663903c
dd5789f
8d928ba
0838e38
5eb58b2
5872798
232acf0
97ffd63
0f1dc02
36d913e
2f02e06
7d81008
e1fa038
0a792ac
bd41cbd
8c1293f
94f17dd
0e2acb3
6976dda
7a1fdec
84cd030
8ce35a2
d683870
b5f61a5
094a540
b3c5629
143eedf
e2a4727
3c2dba5
ce320d2
c1b2d83
a2090a8
3ef98e7
bbf11d7
974039c
c7b2419
e5009f9
8250a1c
6a93ff6
b8fcd20
cb8308c
2269fd7
3cc6673
10427eb
1b40759
ce46baa
d870a9b
dcf7e87
db11996
db1cb02
1d902ff
42a9b76
aa5e2da
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Love it!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit. You can decorate this function with @cached_property
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps this doesn't need to be a property at all since it's only used in the
execute()
method?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is some work going on to standardize the hook access in Amazon provider package. See #29001. I agree with you it is not necessary to store the hook in a property but (and this is only my personal opinion), using @cached_property makes the code cleaner
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@vincbeck I have pushed some changes, please let me know if it's fine now