search

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-1521][FOLLOWUP] Move BasicPrincipal to spi module #2665

Closed turboFei closed 1 month ago

turboFei commented 1 month ago

What changes were proposed in this pull request?

This PS is a followup of CELEBORN-1521, move the BasicPrincipal in to celeborn-spi module. so that customer do not need to implement it by themselves.

Why are the changes needed?

For authentication extension.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Existing GA.

turboFei commented 1 month ago

cc @pan3793

pan3793 commented 1 month ago

move the BasicPrincipal in to celeborn-spi module

I'm not against this, but does it have benefits?

remove the constructor with CelebornConf in HttpAuthenticationFactory as CelebornConf is in celeborn-common module and not deployed for developer.

I would keep this, the developer still has the option to write custom plugins in their forked celeborn repo

turboFei commented 1 month ago

move the BasicPrincipal in to celeborn-spi module

I'm not against this, but does it have benefits?

The only benefit is that, customer do not need to implement java.security.Principal by them, that is all

pan3793 commented 1 month ago

The only benefit is that, customer do not need to implement java.security.Principal by them, that is all

sounds reasonable

pan3793 commented 1 month ago

Merged to main