Skip to content

Fix NPE in S3PinotFS credential refresh in init#18679

Merged
xiangfu0 merged 1 commit into
apache:masterfrom
deepthi912:fix-s3pinotfs-credential-refresh-npe
Jun 4, 2026
Merged

Fix NPE in S3PinotFS credential refresh in init#18679
xiangfu0 merged 1 commit into
apache:masterfrom
deepthi912:fix-s3pinotfs-credential-refresh-npe

Conversation

@deepthi912
Copy link
Copy Markdown
Collaborator

  • S3PinotFS was built via the 3-arg init (e.g., by callers that supply their own pre-configured
    S3Client).
  • The retry path calls initOrRefreshS3Client(), which immediately dereferences the null _s3Config

@deepthi912 deepthi912 requested a review from rhodo June 4, 2026 07:27
@deepthi912 deepthi912 added tiered-storage Related to tiered storage support configuration Config changes (addition/deletion/change in behavior) labels Jun 4, 2026
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Jun 4, 2026

Codecov Report

❌ Patch coverage is 0% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 64.46%. Comparing base (dd6520c) to head (a8d62ed).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
.../org/apache/pinot/plugin/filesystem/S3PinotFS.java 0.00% 4 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master   #18679      +/-   ##
============================================
- Coverage     64.47%   64.46%   -0.01%     
  Complexity     1291     1291              
============================================
  Files          3371     3371              
  Lines        208551   208551              
  Branches      32569    32569              
============================================
- Hits         134455   134438      -17     
- Misses        63292    63305      +13     
- Partials      10804    10808       +4     
Flag Coverage Δ
custom-integration1 100.00% <ø> (ø)
integration 100.00% <ø> (ø)
integration1 100.00% <ø> (ø)
integration2 0.00% <ø> (ø)
java-21 64.46% <0.00%> (-0.01%) ⬇️
temurin 64.46% <0.00%> (-0.01%) ⬇️
unittests 64.46% <0.00%> (-0.01%) ⬇️
unittests1 56.89% <ø> (-0.02%) ⬇️
unittests2 37.09% <0.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@xiangfu0 xiangfu0 merged commit 5c70d88 into apache:master Jun 4, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

configuration Config changes (addition/deletion/change in behavior) tiered-storage Related to tiered storage support

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants