Issue 7459: Hadoop can't create S3 client with latest AWS SDK by rtjd6554 · Pull Request #7520 · gchq/sleeper

rtjd6554 · 2026-06-24T10:26:03Z

Make sure you have checked all steps below.

Issue

My PR fully resolves the following issues. I've referenced an issue in the PR title, for example "Issue 1234 - My
Feature". Note that before an issue is finished, you can still make a pull request by raising a separate issue
for your progress.
- Resolves Hadoop can't create S3 client with latest AWS SDK #7459

Tests

My PR adds the following tests based on our test strategy OR does not need testing for this extremely good reason:
- Execution of existing test suite, primary focus has been around the integration tests that utilise s3client within it.
  E.g. DockerInstanceIT, TpchSchemaPartitionedInsertPartitioningEnabledIT or DataFusionCompactionRunnerLocalStackIT

Documentation

In case of new functionality, my PR adds documentation that describes how to use it, or I have linked to a
separate issue for that below.
If I have added new Java code, I have added Javadoc that explains it following our conventions and style.
If I have added or removed any dependencies from the project, I have updated the NOTICES file.

ca61688 · 2026-07-02T07:47:19Z

There's a lot of code added here, I know the ticket says the existing test covers everything for this ticket but are there any unit tests that could be added for this class?

There is a lot of coverage of this already. It's used in all the integration tests for code that uses Hadoop, although that's just against LocalStack.

I think if we run the full performance test suite against this branch before merging that's probably enough?

I think the PR description needs updating to clarify which tests have been run or need to be run.

I think we do need to run the performance test suite against this branch before we can merge it, because we don't know how this affects performance, or if it's working for Spark in a deployed instance.

Thinking about it, the behaviour of applying the Hadoop configuration properties would be good to have unit tests for. Can we do that?

If I could see a direct one-to-one correspondence with the Hadoop equivalent of this class I'd be less worried. As it is, the diff between this and the Hadoop version isn't easily comparable.

Would you prefer to unit test the behaviour or make it a more direct copy of the Hadoop code?

Here's a link to the original thread if you're reading this in the review: #7520 (comment)

Unit tests added

ca61688 · 2026-07-02T07:47:56Z

Overall looks good, one comment about if we can add more tests

patchwork01

In the AWS SDK release notes it states that the change was to remove the apache-client dependency by default. That contains the HTTP client used by Hadoop, and it still exists:

https://github.com/aws/aws-sdk-java-v2/releases/tag/2.46.0

I think it would be much safer and easier to add that back in, rather than carry on in the current direction. The original idea was to replace a minimum of the Hadoop code with something that will do the same thing. If that's not necessary I think we're better off not doing it anyway, and this is taking too long to match Hadoop.

I think it looks like all we need is the apache-client dependency in the parquet module.

It might be best to make a fresh branch for that and leave this here in case we want to pick it back up?

patchwork01 · 2026-07-31T10:20:09Z

+
+    @Override
+    public S3Client createS3Client(URI uri, S3ClientCreationParameters params) throws IOException {
+        return configureClientBuilder(S3Client.builder(), params, getConf(), uri.getHost())


It doesn't look like this has been addressed. Here's a link to the original comment: #7520 (comment)

patchwork01 · 2026-07-31T10:23:25Z

+    }
+
+    @Test
+    void shouldApplyRequestTimeoutFromConfiguration() throws Exception {


The comment doesn't explain the correspondence to the Hadoop code I mentioned. It's testing that AWSClientConfig.createClientConfigBuilder is used.

patchwork01 · 2026-07-31T10:24:03Z

@@ -47,10 +47,21 @@
            <artifactId>hadoop-aws</artifactId>
        </dependency>
        <!-- We exclude the AWS SDK as a dependency of hadoop-aws because it uses the bundle, so add just S3 back in. -->


This comment needs updating. Most of it is still correct but we're not just adding S3 back in any more.

patchwork01 · 2026-07-31T10:26:28Z

+            clientOverrideConfigBuilder.addMetricPublisher(
+                    new AwsStatisticsCollector(parameters.getMetrics()));
+        }
+


That's not what the Hadoop implementation does. Is there a reason to make this different? Here's a link to the original thread: #7520 (comment)

patchwork01 · 2026-07-31T10:29:52Z

+        configureEndpointAndRegion(builder, parameters, conf);
+
+        S3Configuration serviceConfiguration = S3Configuration.builder()
+                .pathStyleAccessEnabled(parameters.isPathStyleAccess())


It doesn't look like this has been done. Here's a link to the original thread: #7520 (comment)

patchwork01 · 2026-07-31T10:31:28Z

+
+    private static final String REQUESTER_PAYS_HEADER_VALUE = "requester";
+
+    private static Region defaultRegion = Region.US_EAST_2;


This can be final.

patchwork01 · 2026-07-31T10:41:45Z

Please see my comment at the start of the review rather than the individual review comments. Those are there in case we still need to replace the Hadoop implementation.

patchwork01 · 2026-08-03T09:57:16Z

Closing as this has been resolved by the following PR:

Issue 7459: Hadoop can't create S3 client with latest AWS SDK #7820

rtjd6554 and others added 21 commits June 24, 2026 06:52

7459: Update sdk version

5e57abe

7459: Implmentation of abstract methods and S3Factory

0b7a429

Merge branch 'develop' into 7459-Hadoop-S3-Client

d080d1c

7459: Improve class name

963357e

7459: Relocate new class

ffd686c

7459: Update imports

fc35926

7459: Inclusion in configuration provider

d3bd3a2

Merge branch 'develop' into 7459-Hadoop-S3-Client

8f49c79

Merge branch 'develop' into 7459-Hadoop-S3-Client

5cb1680

Merge branch 'develop' into 7459-Hadoop-S3-Client

12022c9

7389: Hard code value for build test

e6c19f0

Merge branch 'develop' into 7459-Hadoop-S3-Client

308d30f

Merge branch 'develop' into 7459-Hadoop-S3-Client

e2b473b

Update version number

f6a6183

re-add missing section

978ecdf

7459: Adjust region setting

67606ef

7459: Adjust implementation

9568ef5

Merge branch 'develop' into 7459-Hadoop-S3-Client

41ad961

7459: Tidy up code

0dc7756

7459: Comment out for test simplify

5a57f93

Re-enable functions

f773b97

rtjd6554 added the needs-reviewer Pull requests that need a reviewer to be assigned label Jul 1, 2026

rtjd6554 marked this pull request as ready for review July 1, 2026 12:01

Update aws-java-sdk-v2.version

1215a87

ca61688 self-assigned this Jul 2, 2026

ca61688 reviewed Jul 2, 2026

View reviewed changes

ca61688 removed their assignment Jul 2, 2026

patchwork01 requested changes Jul 2, 2026

View reviewed changes

Comment thread java/common/parquet/src/main/java/sleeper/parquet/HadoopS3ClientFactory.java

Comment thread java/common/parquet/src/main/java/sleeper/parquet/HadoopS3ClientFactory.java

Comment thread java/common/parquet/src/main/java/sleeper/parquet/HadoopS3ClientFactory.java

patchwork01 assigned rtjd6554 Jul 2, 2026

patchwork01 removed the needs-reviewer Pull requests that need a reviewer to be assigned label Jul 20, 2026

patchwork01 assigned rtjd6554 Jul 20, 2026

rtjd6554 added 5 commits July 20, 2026 10:18

Review comments addressed

f7ce8ca

7459: Update to add default region

115272a

7459: Change default region

9e5a90b

Update to latest version of sdkv2

fc36067

Re-add missing elements from review

11a35aa

rtjd6554 mentioned this pull request Jul 22, 2026

Bump aws-java-sdk-v2.version from 2.45.1 to 2.49.0 in /java #7724

Closed

rtjd6554 and others added 10 commits July 22, 2026 07:33

Merge branch 'develop' into 7459-Hadoop-S3-Client

1339eb6

Update to latest version of library

7c385a0

Merge branch 'develop' into 7459-Hadoop-S3-Client

f8aa186

7459: Add retry strategy

da562bc

Merge branch 'develop' into 7459-Hadoop-S3-Client

afbd3fb

Merge branch 'develop' into 7459-Hadoop-S3-Client

b1c7ace

Update version number

a6fcf0c

7459: Set http client builder with defaults

b8f87cc

Merge branch 'develop' into 7459-Hadoop-S3-Client

13b8979

Update sdkv2 to latest

cf2bf46

rtjd6554 mentioned this pull request Jul 30, 2026

Bump aws-java-sdk-v2.version from 2.45.1 to 2.49.6 in /java #7804

Closed

7459: Adjust httpbuilders

e2d2221

rtjd6554 removed their assignment Jul 31, 2026

rtjd6554 added the needs-reviewer Pull requests that need a reviewer to be assigned label Jul 31, 2026

patchwork01 reviewed Jul 31, 2026

View reviewed changes

patchwork01 removed the needs-reviewer Pull requests that need a reviewer to be assigned label Jul 31, 2026

patchwork01 assigned rtjd6554 Jul 31, 2026

rtjd6554 added the on-hold label Aug 3, 2026

patchwork01 closed this Aug 3, 2026

patchwork01 deleted the 7459-Hadoop-S3-Client branch August 3, 2026 09:57


		private static final String REQUESTER_PAYS_HEADER_VALUE = "requester";

		private static Region defaultRegion = Region.US_EAST_2;

Uh oh!

Conversation

rtjd6554 commented Jun 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Issue

Tests

Documentation

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

patchwork01 Jul 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ca61688 commented Jul 2, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

patchwork01 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

patchwork01 Jul 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

patchwork01 Jul 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

patchwork01 Jul 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

patchwork01 commented Jul 31, 2026

Uh oh!

patchwork01 commented Aug 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

rtjd6554 commented Jun 24, 2026 •

edited

Loading

patchwork01 Jul 17, 2026 •

edited

Loading

patchwork01 left a comment •

edited

Loading

patchwork01 Jul 31, 2026 •

edited

Loading

patchwork01 Jul 31, 2026 •

edited

Loading

patchwork01 Jul 31, 2026 •

edited

Loading