-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Stripping Input Data from Background Callback Polling #3113
base: dev
Are you sure you want to change the base?
Conversation
@T4rk1n can you please review? thx |
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.
Looks good overall. I just have a question if we could just do body = "{}"
if we have the cache key instead of setting the inputs to null, think that might require some work on the backend callback but wouldn't it be better?
This would indeed take much more effort to unwind how the background callbacks function to completely strip the need for the input structure, as this is used to verify the callback structure, as well as pull things like context (I'm assuming for using things like progress updates) If we want to do this in a rework, it should probably be done with my other open PR, as this will require a much more lengthy refactor. This PR however wouldnt require a rework and would solve the issue with lengthy data being passed to the backend when unnecessary. |
It's fine like this, I don't think the refactor is really worth the effort for now. |
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.
💃 Looks good, thank you.
Attempting to make polling requests not eat bandwidth
This is to fix #3111