Skip to content

updated putNodeData and zookeeperPutData to pass data in the payload/…#16809

Merged
xiangfu0 merged 1 commit intoapache:masterfrom
robertzych:fix-zookeeper-put
Sep 13, 2025
Merged

updated putNodeData and zookeeperPutData to pass data in the payload/…#16809
xiangfu0 merged 1 commit intoapache:masterfrom
robertzych:fix-zookeeper-put

Conversation

@robertzych
Copy link
Copy Markdown
Contributor

…body instead of a query param

This screenshot shows that zk node data is now being passed via the request payload:

Screenshot 2025-09-12 at 6 05 21 PM

This fixes issue #15311

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Sep 13, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 63.36%. Comparing base (93de8f0) to head (2bdfae7).
⚠️ Report is 951 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master   #16809      +/-   ##
============================================
- Coverage     63.36%   63.36%   -0.01%     
  Complexity     1399     1399              
============================================
  Files          3055     3055              
  Lines        178916   178916              
  Branches      27419    27419              
============================================
- Hits         113377   113362      -15     
- Misses        56789    56807      +18     
+ Partials       8750     8747       -3     
Flag Coverage Δ
custom-integration1 100.00% <ø> (ø)
integration 100.00% <ø> (ø)
integration1 100.00% <ø> (ø)
integration2 0.00% <ø> (ø)
java-11 63.34% <ø> (+7.04%) ⬆️
java-21 63.33% <ø> (-0.01%) ⬇️
temurin 63.36% <ø> (-0.01%) ⬇️
unittests 63.35% <ø> (-0.01%) ⬇️
unittests1 56.31% <ø> (-0.02%) ⬇️
unittests2 33.39% <ø> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@xiangfu0 xiangfu0 requested a review from Copilot September 13, 2025 18:06
@xiangfu0 xiangfu0 added enhancement Improvement to existing functionality ui UI related issue labels Sep 13, 2025
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Updates Zookeeper node data operations to pass data in the request payload instead of as query parameters. This change addresses an issue where large Zookeeper node data was being passed through query parameters, which can cause issues with URL length limits and is not the proper approach for data payloads.

  • Modified putNodeData to extract data from parameters and pass it as request body
  • Updated zookeeperPutData to accept optional data parameter for request payload
  • Restructured parameter handling to separate query parameters from data payload

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
pinot-controller/src/main/resources/app/utils/PinotMethodUtils.ts Refactored putNodeData to separate data from query parameters and pass data to request body
pinot-controller/src/main/resources/app/requests/index.ts Updated zookeeperPutData function signature to accept optional data parameter for request payload

Copy link
Copy Markdown
Contributor

@xiangfu0 xiangfu0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, thanks for adding this.
Now we could update larger payload through ui :)

@xiangfu0 xiangfu0 merged commit c897df7 into apache:master Sep 13, 2025
16 of 18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Improvement to existing functionality ui UI related issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants