-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Support MinionContext AuthProvider in Minion executors #17405
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
base: master
Are you sure you want to change the base?
Conversation
fadc3b0 to
79edde2
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #17405 +/- ##
============================================
+ Coverage 63.28% 63.30% +0.02%
- Complexity 1474 1476 +2
============================================
Files 3154 3162 +8
Lines 188007 188595 +588
Branches 28782 28858 +76
============================================
+ Hits 118977 119391 +414
- Misses 59807 59955 +148
- Partials 9223 9249 +26
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
79edde2 to
e7190b9
Compare
...c/main/java/org/apache/pinot/plugin/minion/tasks/BaseMultipleSegmentsConversionExecutor.java
Outdated
Show resolved
Hide resolved
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.
Pull request overview
This PR enables Minion task executors to use a dynamic runtime AuthProvider from MinionContext instead of relying solely on static task tokens. This allows per-request token rotation, custom authorization headers, and better support for short-lived authentication tokens.
Key Changes:
- Executors now check
MinionContext.getTaskAuthProvider()before falling back to the legacyAUTH_TOKENconfiguration - Three locations updated:
BaseSingleSegmentConversionExecutor,BaseMultipleSegmentsConversionExecutor(two call sites), andSegmentUploadContext - Tests verify runtime provider preference and legacy fallback behavior
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
BaseMultipleSegmentsConversionExecutorTest.java |
Added test teardown cleanup and two tests verifying runtime provider preference and task token fallback |
BaseSingleSegmentConversionExecutor.java |
Updated executeTask to prefer runtime AuthProvider over static token with NullAuthProvider check |
BaseMultipleSegmentsConversionExecutor.java |
Applied runtime provider logic in preProcess, executeTask, and SegmentUploadContext constructor |
.../src/main/java/org/apache/pinot/plugin/minion/tasks/BaseSingleSegmentConversionExecutor.java
Outdated
Show resolved
Hide resolved
...c/main/java/org/apache/pinot/plugin/minion/tasks/BaseMultipleSegmentsConversionExecutor.java
Outdated
Show resolved
Hide resolved
31ce6f0 to
1ab277b
Compare
Minion’s executors previously rebuilt auth from a static task token, forcing the Authorization header and preventing per-request token rotation. This broke short‑lived tokens and custom header requirements.