apache / celeborn

Apache Celeborn is an elastic and high-performance service for shuffle and spilled data.
https://celeborn.apache.org/
Apache License 2.0
862 stars 351 forks source link

[CELEBORN-1553] Using the request base url as swagger server #2674

Closed turboFei closed 1 week ago

turboFei commented 1 month ago

What changes were proposed in this pull request?

Using the request base url as swagger server, to prevent the swagger server not reachable and CORS error if the swagger server urls do not match.

Currently, if the http host is bound to local, the swagger server is not reachable. For example:

celeborn.master.http.host=0.0.0.0
celeborn.worker.http.host=0.0.0.0

Why are the changes needed?

image

Does this PR introduce any user-facing change?

No, just use the request base url as swagger server.

How was this patch tested?

Integration testing:

image image
codecov[bot] commented 1 month ago

Codecov Report

Attention: Patch coverage is 0% with 4 lines in your changes missing coverage. Please review.

Project coverage is 33.35%. Comparing base (ea6617c) to head (e4eddab). Report is 17 commits behind head on main.

Files Patch % Lines
...rver/common/http/api/CelebornOpenApiResource.scala 0.00% 3 Missing :warning:
...rg/apache/celeborn/server/common/HttpService.scala 0.00% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #2674 +/- ## ========================================== - Coverage 39.83% 33.35% -6.47% ========================================== Files 239 310 +71 Lines 15026 18193 +3167 Branches 1362 1672 +310 ========================================== + Hits 5984 6067 +83 - Misses 8711 11786 +3075 - Partials 331 340 +9 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

turboFei commented 1 month ago

cc @pan3793

turboFei commented 3 weeks ago

also cc @cfmcgrady @SteNicholas

turboFei commented 1 week ago

Hi @RexXiong

If no more comments, can we merge it? thanks

RexXiong commented 1 week ago

Hi @RexXiong

If no more comments, can we merge it? thanks

Thanks for the reminder. Merge to main(v0.6.0)