Skip to content

[CELEBORN-1317][FOLLOWUP] Improve parameters, description and document of REST API#2495

Closed
SteNicholas wants to merge 2 commits intoapache:mainfrom
SteNicholas:CELEBORN-1317
Closed

[CELEBORN-1317][FOLLOWUP] Improve parameters, description and document of REST API#2495
SteNicholas wants to merge 2 commits intoapache:mainfrom
SteNicholas:CELEBORN-1317

Conversation

@SteNicholas
Copy link
Member

@SteNicholas SteNicholas commented May 8, 2024

What changes were proposed in this pull request?

Improve parameters, description and document of Celeborn REST API, including:

  1. The POST request uses @FormParam instead of @QueryParam.
  2. The parameter name uses lowercase instead of uppercase.
  3. The description of /exclude aligns with document in monitoring.md.
  4. The document of REST API adds the Method and Parameters to document GET/POST method and corresponding interface.

Why are the changes needed?

The parameters, description and document of REST API need to improve after http server refine.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

GA.

@SteNicholas SteNicholas marked this pull request as draft May 8, 2024 07:44
@SteNicholas SteNicholas marked this pull request as ready for review May 8, 2024 08:02
@SteNicholas SteNicholas requested a review from RexXiong May 8, 2024 08:12
@SteNicholas
Copy link
Member Author

Ping @turboFei, @RexXiong. PTAL.

@SteNicholas SteNicholas requested a review from turboFei May 9, 2024 05:09
@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 40.78%. Comparing base (121395f) to head (d66232e).
Report is 28 commits behind head on main.

❗ Current head d66232e differs from pull request most recent head ba0051b. Consider uploading reports for the commit ba0051b to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2495      +/-   ##
==========================================
+ Coverage   40.17%   40.78%   +0.61%     
==========================================
  Files         218      220       +2     
  Lines       13742    13962     +220     
  Branches     1214     1241      +27     
==========================================
+ Hits         5520     5693     +173     
- Misses       7905     7940      +35     
- Partials      317      329      +12     

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

Copy link
Contributor

@RexXiong RexXiong left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@RexXiong
Copy link
Contributor

RexXiong commented May 9, 2024

Merge to main(v0.5.0)

@RexXiong RexXiong closed this in db163bd May 9, 2024
pan3793 pushed a commit that referenced this pull request Jul 11, 2024
…ded APIs

### What changes were proposed in this pull request?

This PR is a follow up for #2495, fix the media types.

### Why are the changes needed?

The media types shown in the swagger UI are not correct.

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?

Before:
<img width="1439" alt="image" src="https://github.com/apache/celeborn/assets/6757692/f287c02b-791c-4677-93b7-ac9c5e4ee34f">
After:
<img width="1341" alt="image" src="https://github.com/apache/celeborn/assets/6757692/13e5d310-7c97-4872-9496-f9b12113b7ab">

Closes #2616 from turboFei/form_app.

Authored-by: Fei Wang <fwang12@ebay.com>
Signed-off-by: Cheng Pan <chengpan@apache.org>
pan3793 pushed a commit that referenced this pull request Jul 11, 2024
…ded APIs

### What changes were proposed in this pull request?

This PR is a follow up for #2495, fix the media types.

### Why are the changes needed?

The media types shown in the swagger UI are not correct.

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?

Before:
<img width="1439" alt="image" src="https://github.com/apache/celeborn/assets/6757692/f287c02b-791c-4677-93b7-ac9c5e4ee34f">
After:
<img width="1341" alt="image" src="https://github.com/apache/celeborn/assets/6757692/13e5d310-7c97-4872-9496-f9b12113b7ab">

Closes #2616 from turboFei/form_app.

Authored-by: Fei Wang <fwang12@ebay.com>
Signed-off-by: Cheng Pan <chengpan@apache.org>
(cherry picked from commit bd3f823)
Signed-off-by: Cheng Pan <chengpan@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants