Support MinionContext AuthProvider in Minion executors#17405
Support MinionContext AuthProvider in Minion executors#17405Jackie-Jiang merged 4 commits intoapache:masterfrom
Conversation
fadc3b0 to
79edde2
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #17405 +/- ##
============================================
- Coverage 63.28% 63.26% -0.02%
- Complexity 1474 1480 +6
============================================
Files 3154 3170 +16
Lines 188007 189512 +1505
Branches 28782 28998 +216
============================================
+ Hits 118977 119900 +923
- Misses 59807 60305 +498
- Partials 9223 9307 +84
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
There was a problem hiding this comment.
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 |
31ce6f0 to
1ab277b
Compare
|
@tarun11Mavani @swaminathanmanish @xiangfu0 can you ptal? |
Jackie-Jiang
left a comment
There was a problem hiding this comment.
I'd do it in a different way, where each task can override the auth token if necessary.
Currently auth token is set when starting a task (in TaskFactoryRegistry.runInternal()). Instead, we shouldn't set it, but check if that is explicitly provided. When explicitly provided, use it as the auth provider; if not, fall back to the minion global one.
59bd7e0 to
c35cd90
Compare
c35cd90 to
3a9a543
Compare
Good catch, thanks. Fixed it to check for explicit token first. |
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.