-
Notifications
You must be signed in to change notification settings - Fork 37
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
Return batch statuses even when some timeout #1815
Merged
ameliabradley
merged 3 commits into
Cargill:main
from
ameliabradley:return-batch-statuses-even-when-some-timeout
Apr 29, 2022
Merged
Return batch statuses even when some timeout #1815
ameliabradley
merged 3 commits into
Cargill:main
from
ameliabradley:return-batch-statuses-even-when-some-timeout
Apr 29, 2022
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ameliabradley
requested review from
agunde406,
Caleb-Hill,
davececchi,
dnewh,
isabeltomb,
jsmitchell,
peterschwarz,
rbuysse,
shannynalayna and
vaporos
as code owners
February 10, 2022 17:21
github-actions
bot
added
backport-triage
It has not yet been determined if this branch should be backported.
main
labels
Feb 10, 2022
ameliabradley
force-pushed
the
return-batch-statuses-even-when-some-timeout
branch
from
February 10, 2022 18:45
bcfbf5b
to
89cbae3
Compare
ameliabradley
force-pushed
the
return-batch-statuses-even-when-some-timeout
branch
from
February 14, 2022 15:18
08bc9b2
to
6330c00
Compare
vaporos
previously approved these changes
Feb 22, 2022
peterschwarz
suggested changes
Feb 22, 2022
ameliabradley
force-pushed
the
return-batch-statuses-even-when-some-timeout
branch
3 times, most recently
from
March 2, 2022 16:24
37571d1
to
6439fc9
Compare
peterschwarz
previously approved these changes
Mar 2, 2022
This change modifies the Splinter /batch_statuses endpoint so that it will return batch statuses, even if some of them time out. Batch statuses that time out are returned as "Unknown". Previously, if a single batch status were to timeout, the endpoint would return none at all. The endpoint result would be a non-parsable string error containing the problem batch ids. This change should give users of this endpoint maximal information. Users can use the successful ids, and they can determine programatically which ones timed out. Also updated several tests to validate the new expected responses. Signed-off-by: Lee Bradley <bradley@bitwise.io>
This change updates the wait_for_batches function in the Reqwest client so that it will identify Unknown batches and return an error. Signed-off-by: Lee Bradley <bradley@bitwise.io>
ameliabradley
force-pushed
the
return-batch-statuses-even-when-some-timeout
branch
from
March 3, 2022 17:10
6439fc9
to
11131b5
Compare
Previously, if a batch status response was "Unknown" or "Pending", the message field would not be returned (unlike "Invalid", "Valid" and "Committed"). The OpenAPI spec states that message is not an optional return value. This change modifies the serde serialization so that both the "Unknown" and "Pending" enum variants are serialized as empty sequences, and thus message is never empty. This change also adds tests to validate the resulting JSON output. To ensure that the string ouput for tests is deterministic, the preserve_order feature is enabled in development builds. Signed-off-by: Lee Bradley <bradley@bitwise.io>
ameliabradley
force-pushed
the
return-batch-statuses-even-when-some-timeout
branch
from
March 3, 2022 21:04
11131b5
to
1ebca2a
Compare
peterschwarz
approved these changes
Mar 4, 2022
vaporos
approved these changes
Apr 29, 2022
peterschwarz
added
backport-0-4
This PR should be backported to the 0-4 branch.
backport-0-6
This PR should be backported to the 0-6 branch.
and removed
backport-triage
It has not yet been determined if this branch should be backported.
labels
Apr 29, 2022
ameliabradley
added
the
backport-complete
This PR has been backported to the appropriate branches
label
May 3, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
backport-0-4
This PR should be backported to the 0-4 branch.
backport-0-6
This PR should be backported to the 0-6 branch.
backport-complete
This PR has been backported to the appropriate branches
main
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This change modifies the Splinter /batch_statuses endpoint so that it
will return batch statuses, even if some of them time out. Batch
statuses that time out are returned as "Unknown".
Previously, if a single batch status were to timeout, the endpoint would
return none at all. The endpoint result would be a non-parsable string
error containing the problem batch ids.
This change should give users of this endpoint maximal information.
Users can use the successful ids, and they can determine programatically
which ones timed out.
Also updated several tests to validate the new expected responses.
Signed-off-by: Lee Bradley bradley@bitwise.io