-
Notifications
You must be signed in to change notification settings - Fork 25
feat: Support for on_success sink in pynumaflow #302
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
Conversation
Signed-off-by: vtiwari5 <[email protected]>
…. Update example added Signed-off-by: vtiwari5 <[email protected]>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #302 +/- ##
=======================================
Coverage 93.85% 93.85%
=======================================
Files 66 66
Lines 3009 3058 +49
Branches 155 159 +4
=======================================
+ Hits 2824 2870 +46
- Misses 135 138 +3
Partials 50 50 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Signed-off-by: Vaibhav Tiwari <[email protected]>
| def with_keys(self, keys: Optional[list[str]]): | ||
| self._keys = keys | ||
| return self | ||
|
|
||
| def with_user_metadata(self, user_metadata: Optional[UserMetadata]): | ||
| self._user_metadata = user_metadata | ||
| return self |
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.
Python's design choice is to return None if a method mutates the object in place (as per std lib methods like list.sort(), list.append(), list.extend(), dict.update(), set.add()).
my_list.sort()
# vs
s_list = sorted(my_list)I'm not sure what to do here since we follow this pattern for Message object in other modules too.
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'll leave it as is for now. I can create a separate issue to fix this, will need to call it out in the next patch release as it is a breaking change for anyone already using previous way.
Signed-off-by: Vaibhav Tiwari <[email protected]>
Signed-off-by: Vaibhav Tiwari <[email protected]>
This PR aims to add support for on_success sink in pynumaflow (pynumaflow-lite already has it)
Fixes: #298
Testing:
Local Testing
Pipeline spec:
Using rust sdk for logger for onSuccess udsink since server info prevents this python SDK version to be deployed as onSuccess ud sink container.
UD sink logs:
OnSuccess sink logs:
Numa container logs: