sarutak commented on PR #57998:
URL: https://github.com/apache/spark/pull/57998#issuecomment-5359898485
Thanks for addressing all the feedback, @shrirangmhalgi.
Three remaining items:
1. **Rebase onto master**: PR #58018 (`additionalSparkProperties()`) has
been merged. Please rebase to pick up that change
and ensure CI passes on the latest master.
2. **[non-blocking suggestion] Add a test verifying auto-config
integration**: Since `AwsStsCredentialProvider.additionalS
parkProperties()` returns this provider's FQCN, a simple test asserting the
class name matches would catch accidental drif
t (e.g., rename or package move):
```java
@Test
void testClassNameMatchesAutoConfigValue() {
AwsStsCredentialProvider stsProvider = new AwsStsCredentialProvider();
stsProvider.init(Map.of(
"spark.security.oidc.aws.roleArn",
"arn:aws:iam::123456789012:role/test"));
Map<String, String> props = stsProvider.additionalSparkProperties();
assertEquals(
SparkOidcAwsCredentialsProvider.class.getName(),
props.get("spark.hadoop.fs.s3a.aws.credentials.provider"));
}
```
This verifies the contract between the driver-side provider (which declares
the auto-config) and the executor-side provider (which S3A instantiates). Both
live in the same module, so the test is straightforward.
3. **[non-blocking suggestion] Add a cache-hit test**:
`FileTokenIngestorSuite` has `loadUsesCachedResultWhenFileUnchanged` which
verifies that repeated calls with unchanged content return the same cached
instance (`assertSame`). A similar test for `SparkOidcAwsCredentialsProvider`
would verify that repeated calls with the same version return the cached
`AwsSessionCredentials` without re-deserialization:
```java
@Test
void testCacheHitReturnsSameInstanceWhenVersionUnchanged() {
populateStore("key-1", "secret-1", "token-1", 1L);
SparkOidcAwsCredentialsProvider provider = new
SparkOidcAwsCredentialsProvider();
AwsCredentials result1 = provider.resolveCredentials();
AwsCredentials result2 = provider.resolveCredentials();
// Same cached instance proves deserialization was skipped on second call
assertSame(result1, result2);
}
```
This aligns with the existing testing pattern in `FileTokenIngestorSuite`
for the same module's caching behavior.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
