Batch Workaround for Deserialization of Long Properties #40301
Merged
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
skapur12
commented
May 22, 2024
| Assertions.assertEquals(OffsetDateTime.parse("2022-01-01T00:00:00Z"), stats.getStartTime()); | ||
| Assertions.assertEquals(OffsetDateTime.parse("2022-01-01T01:00:00Z"), stats.getLastUpdateTime()); | ||
| Assertions.assertEquals(Duration.parse("PT1H"), stats.getUserCpuTime()); | ||
| Assertions.assertEquals(Duration.parse("PT30M"), stats.getKernelCpuTime()); |
There was a problem hiding this comment.
I added assertions to check the values of all of the properties in every class, but I'm wondering if that might be overkill—Would it be better to just assert the values of the long properties only? (e.g. only readIOps, writeIOps, numSucceededTasks, numFailedTasks, numTaskRetries in this case)
|
API change check APIView has identified API level changes in this PR and created following API reviews. |
skapur12
force-pushed
the
skapur/long-workaround
branch
from
85cb7a0 to
364ec10
Compare
wiboris
previously requested changes
May 28, 2024
skapur12
force-pushed
the
skapur/long-workaround
branch
from
364ec10 to
6cd6e00
Compare
skapur12
force-pushed
the
skapur/long-workaround
branch
from
6cd6e00 to
d436673
Compare
dpwatrous
approved these changes
Jun 7, 2024
skapur12
dismissed
wiboris’s stale review
Addressed comment and updated the test recordings
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description
There is a bug in the Batch Service: all properties defined as longs in our TypeSpec are returned from the service side as strings.
The service team is working on a fix for this issue in our next API release.
In the meantime, this PR temporarily resolves the bug by:
Once the new API is released on the service side, we will revert these temporary deserialization changes and verify that the service side logic works with the test coverage added in this PR.
If an SDK is being regenerated based on a new swagger spec, a link to the pull request containing these swagger spec changes has been included above.
All SDK Contribution checklist:
General Guidelines and Best Practices
Testing Guidelines