-
Notifications
You must be signed in to change notification settings - Fork 286
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 @track(flush=True) #193
Conversation
43bf9b9
to
68ae5b5
Compare
68ae5b5
to
b209cf1
Compare
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.
LGTM, but let's get input from @alexkuzmik @japdubengsub
@@ -180,6 +187,9 @@ def wrapper(*args, **kwargs) -> Any: # type: ignore | |||
output=result, | |||
capture_output=capture_output, | |||
) | |||
if flush: |
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.
@dsblank this will not work for generators. See the code above, if the generator was decorated, we return wrapped generator before you are calling flush()
.
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.
Is there anything we can do to flush after the generator is done?
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.
we can do flushing in _after_call
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.
Ok, thanks for the info! I'll revise the PR and flush in the _after_call.
@dsblank Closing this PR for now and we can re-open when it's ready |
Details
This PR adds @track(flush=True) so that you don't have to do it manually after the function has been called.
Before:
After:
Issues
None, except for adding some convenience.
I'm not sure if one can flush even async decorators inside the decorator?
Testing
No tests added yet. Just seeing if you think this would be worth while first.
Documentation
None yet.