-
Notifications
You must be signed in to change notification settings - Fork 230
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
Allow SDK to handle speculative WFT with command events #1509
Allow SDK to handle speculative WFT with command events #1509
Conversation
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.
Makes sense to me, just minor comment changes
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 admit not understanding this very much, but is it possible to have an integration test that would break without this change but works now?
No, the server won't trigger this code path yet. |
Arguably this dead code shouldn't make it into the repo until it can be exercised (I fear adding code that we can't confirm works in end-to-end in CI). But if we're confident there are no dangers in adding this and it'll just remain dead code for now and it won't start breaking when server does turn something on, no prob. Is there a server PR that, when released, will trigger this? Would like us to track the server work to make sure we can add the integration test when we can. |
This code is exercised by unit tests and is called in integration tests as well. There is a clear contract between the SDK and Server we are verifying the SDK side satisfies it's side of the contract.
Agree, the server work is tracked in Jira, but I'll open up a feature issue for testing this scenario across all SDKs. |
Allow SDK to handle speculative WFT with command events. Originally I thought this change was going to be quite bad, but I realized I didn't need to touch the cacheing logic and could keep it all in the history iteration.