-
Notifications
You must be signed in to change notification settings - Fork 919
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
[Console] Enhance support for JSON with long numerals #5130
[Console] Enhance support for JSON with long numerals #5130
Conversation
* Improve testing of clients with long numerals support * Work around serializer inheritance in the JS client Signed-off-by: Miki <[email protected]>
…#4562)(opensearch-project#5130) Also: * Add support for parsing and stringifying JSON with long numerals into `@osd/std` * Upgrade to `@opensearch/[email protected]` which supports long numerals * Add support for long numerals to `http/fetch` * Improve testing of clients with long numerals support Signed-off-by: Miki <[email protected]>
Codecov Report
@@ Coverage Diff @@
## main #5130 +/- ##
=======================================
Coverage 66.50% 66.50%
=======================================
Files 3403 3403
Lines 65026 65030 +4
Branches 10401 10403 +2
=======================================
+ Hits 43243 43246 +3
- Misses 19214 19294 +80
+ Partials 2569 2490 -79
Flags with carried forward coverage won't be shown. Click here to find out more.
|
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.
@AMoo-Miki I don't have enough expertise to really evaluate, but it seems fine.
However, the increased complexity of supporting all these mixed modes seems like it might be worth going back to first principles to see if there's a way we can simplify our JSON handling overall.
Can you elaborate on why we can't fix this in the JS client instead? Is it semver related? |
That was exactly what I was thinking when I added the serialization and deserialization to |
We create instances of A On the other hand, If I was the JS client and I was asked to change the inheritance, I would reply that a |
Also: * Add support for parsing and stringifying JSON with long numerals into `@osd/std` * Upgrade to `@opensearch/[email protected]` which supports long numerals * Add support for long numerals to `http/fetch` * Improve testing of clients with long numerals support Signed-off-by: Miki <[email protected]>
The backport to
To backport manually, run these commands in your terminal: # Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/OpenSearch-Dashboards/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/OpenSearch-Dashboards/backport-2.x
# Create a new branch
git switch --create backport/backport-5130-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 588efcd887f5917e5aca6593ffae1a8e10629d35
# Push it to GitHub
git push --set-upstream origin backport/backport-5130-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/OpenSearch-Dashboards/backport-2.x Then, create a pull request where the |
The backport to
To backport manually, run these commands in your terminal: # Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/OpenSearch-Dashboards/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/OpenSearch-Dashboards/backport-2.x
# Create a new branch
git switch --create backport/backport-5130-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 588efcd887f5917e5aca6593ffae1a8e10629d35
# Push it to GitHub
git push --set-upstream origin backport/backport-5130-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/OpenSearch-Dashboards/backport-2.x Then, create a pull request where the |
should not be backported |
…ject#5130) * Improve testing of clients with long numerals support * Work around serializer inheritance in the JS client Signed-off-by: Miki <[email protected]> Signed-off-by: Leo Deng <[email protected]>
…ject#5130) * Improve testing of clients with long numerals support * Work around serializer inheritance in the JS client Signed-off-by: Miki <[email protected]> Signed-off-by: Willie Hung <[email protected]>
…ject#5130) * Improve testing of clients with long numerals support * Work around serializer inheritance in the JS client Signed-off-by: Miki <[email protected]>
Description
Enhance support for JSON with long numerals: Followup to #4562
Check List
yarn test:jest
yarn test:jest_integration
yarn test:ftr