-
Notifications
You must be signed in to change notification settings - Fork 13
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
0.14 Development #387
0.14 Development #387
Conversation
…store-apis Implement Object Storage APIs
…-basic-checkpoints Working Basic Checkpointing
S3 Object Storage
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #387 +/- ##
==========================================
+ Coverage 98.04% 98.17% +0.12%
==========================================
Files 149 150 +1
Lines 5471 5857 +386
==========================================
+ Hits 5364 5750 +386
Misses 107 107
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
TTL Checkpointing
Cleanup Deprecrated File APIs
…tors Checkpoint Implementations for Athena and DynamoDB
…rable Make Storage Configurable
Add Schema Enforcement Module
rename checkpoint-storage-backend to storage-backend
# DEPRECATED CODE BELOW ## | ||
# | ||
# The classes below are slated to be removed in the future. | ||
# Additionally, there are aliases from the old class names to the new class | ||
# names to ensure backwards compatibility. These aliases will be removed in | ||
# the future. |
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.
In the future, I think it might be helpful to use
def __init__(self):
warnings.warn(message, DeprecationWarning, stacklevel=2)
in addition to this warning in order to help IDE users and be a bit noisier about deprecation.
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.
@jbristow keep me honest there if we deprecate something in the future.
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.
One change, one formatting futures comment
def build_schema(self) -> Schema: | ||
return Schema(self._inner.to_schema()) | ||
|
||
class FetchSchema(SchemaEnforcementMode): |
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.
Was it intended for the rest of the SchemaEnforcementMode
subclasses to have docstrings but not this one?
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.
Fixed
nodestream/pipeline/pipeline.py
Outdated
|
||
def __init__(self, steps: Tuple[Step, ...], step_outbox_size: int) -> None: | ||
def __init__( | ||
self, steps: Tuple[Step, ...], step_outbox_size: int, object_store: ObjectStore |
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.
how do we feel about encouraging adding a trailing comma for wrapped parameter lists with 3 or more parameters? (not a hard rule, maybe?)
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.
Fixed
Non
main
branch destination for 0.14 PRs.https://github.com/nodestream-proj/nodestream/milestone/10
When all PRs are merged and issues closed. This PR will move out of draft, final reviewed, and merged.