-
Notifications
You must be signed in to change notification settings - Fork 1
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
4290 prefect pipeline #266
base: master
Are you sure you want to change the base?
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #266 +/- ##
==========================================
- Coverage 91.96% 90.39% -1.57%
==========================================
Files 39 41 +2
Lines 4082 4176 +94
Branches 590 485 -105
==========================================
+ Hits 3754 3775 +21
- Misses 265 334 +69
- Partials 63 67 +4
☔ View full report in Codecov by Sentry. |
e99a1f1
to
f666c6c
Compare
…re into 4290-prefect-pipeline
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.
Just adding my 2c's worth while I look at this.
@@ -0,0 +1,64 @@ | |||
from urllib.parse import urlparse | |||
import boto3 |
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.
Some functionality for handling objects on S3 is already implemented as an S3StorageBroker
class that suports query, download, upload and delete operations. Could that be reused instead of duplicating the functionality here?
""" Override HandlerBase""" | ||
|
||
try: | ||
self._file_checksum = self.etag |
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.
I can't see where this etag
attribute is set?
These modify aodncore to allow for running of pipelines with Prefect and S3 buckets. The changes should not affect existing pipeline operations.