Skip to content

Conversation

@alextwoods
Copy link
Contributor

@alextwoods alextwoods commented Dec 17, 2025

Fix credential reloading by default + gracefully handle missing files

Motivation and Context

Modifications

This PR addresses two, related issues by:

  1. Using a more dynamic supplier in the ProfileFileSupplier.reloadWhenModified method - this ensures that credential provider chains that rely on this method will fallback to the next available provider AND that if the file is eventually created, it can be used.
  2. Replace static/fixedProfileFile default suppliers with the ProfileFileSupplier.defaultSupplier which uses refreshable suppliers which correctly cache ProfileFiles (mitigating the performance impact seen in some earlier versions) while also allowing updates by checking the file modification time.

Q: What about the performance impact?
There were a few previous PRs (#4850 and #3950) that made changes to default profile file suppliers to mitigate performance impacts. However, the performance degredation (loading profileFile on every request) comes from using ProfileFile::defaultProfileFile as a supplier and not from using the ProfileFileSupplier.defaultSupplier. Using ``ProfileFile::defaultProfileFilehas NO caching and no refresh logic - every singleget` call will always re-load the file(s). The `ProfileFileSupplier.defaultSupplier` uses file modification times to ensure that the file(s) are reloaded only when changed.

Benchmark Results
#3950 identified performance issues when running JsonProtocolBenchmark, so we use the same here (since this would load a default client and make many requests):

Benchmark Mode Cnt Score Error Units
JsonProtocolBenchmark.before         thrpt 10 49365.788 ± 2169.444 ops/s
JsonProtocolBenchmark.after          thrpt 10 49959.522 ± 1983.392 ops/s

Testing

New and existing unit tests.

Ran reproduction test cases from both #4268 and #6615.

Screenshots (if appropriate)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

Checklist

  • I have read the CONTRIBUTING document
  • Local run of mvn install succeeds
  • My code follows the code style of this project
  • I have added tests to cover my changes
  • All new and existing tests passed
  • I have added a changelog entry. Adding a new entry must be accomplished by running the scripts/new-change script and following the instructions. Commit the new file created by the script in .changes/next-release with your changes.

License

  • I confirm that this pull request can be released under the Apache 2 license

@alextwoods alextwoods requested a review from a team as a code owner December 17, 2025 01:25
@alextwoods alextwoods added the no-api-surface-area-change Indicate there is no API surface area change and thus API surface area review is not required label Dec 17, 2025
@sonarqubecloud
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
2 Security Hotspots
34.1% Coverage on New Code (required ≥ 80%)
5.9% Duplication on New Code (required ≤ 3%)
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-api-surface-area-change Indicate there is no API surface area change and thus API surface area review is not required

Projects

None yet

2 participants