Add DataLakeFileSystemClient constructor in ADLSFileIO #14966

sarthaksin1857 · 2026-01-05T14:46:47Z

Summary

Added a constructor to ADLSFileIO which allows the creator to pass in a custom DataLakeFileSystemClient as opposed to using the default constructor/auth supported by the library

Reasoning

If an application is constructing ADLSFileIO they may have internal auth mechanisms which cannot be contained in the library. The AWS and GCP implementations of DelegateFileIO have this functionality so its seems fitting to add it to the azure implementation as well

sarthaksin1857 · 2026-01-05T18:13:31Z

@rymurr can you take a look at this PR?

rymurr · 2026-01-07T13:25:08Z

azure/src/main/java/org/apache/iceberg/azure/adlsv2/ADLSFileIO.java

+   */
+  public ADLSFileIO(SerializableSupplier<DataLakeFileSystemClient> clientSupplier) {
+    this.clientSupplier = clientSupplier;
+    this.properties = SerializableMap.copyOf(Maps.newHashMap());


Why do we do this when it isn't done in teh no-op constructor?

I wanted this constructor to be able to be used without calling initialize initialize does a bunch of stuff we don't need to do if we are passing in our own client. To use this class we need azureProperties to be initialized

The API contract is to call initialize though right? Are we sayign this constructor doesn't follow the contract?

https://github.com/sarthaksin1857/iceberg/blob/345140e9f7d428fcc3b4fcc639a690084a698b76/aws/src/main/java/org/apache/iceberg/aws/s3/S3FileIO.java#L128-L141

This constructor for example does not follow the contract either. The contract in my eyes is

If you use the no arg constructor, you have to call initialize before using it

If you use a constructor which has arguments you do not need to call initialize

if that is the case would it be better to call initialize from the constructor. To make the contract more clear and ensure if we change initialize we don't have to change the constructo

rymurr · 2026-01-07T13:26:36Z

azure/src/main/java/org/apache/iceberg/azure/adlsv2/ADLSFileIO.java

-        new DataLakeFileSystemClientBuilder().httpClient(HTTP);
+    if (clientSupplier != null) {
+      return clientSupplier.get();
+    } else {


nit: the else is redundant here and just causes unneccessary whitespace changes below

What about constructors which call this directly instead of via the public method above . I could just remove it from the constructor above though

+1 to removing else, and from the constructor above.
lets keep one place where the clientSupplier is used since everything else calls this method

rymurr · 2026-01-07T13:27:47Z

azure/src/main/java/org/apache/iceberg/azure/adlsv2/ADLSFileIO.java

  public DataLakeFileSystemClient client(String path) {
-    ADLSLocation location = new ADLSLocation(path);
-    return client(location);
+    if (clientSupplier != null) {


I am concerned about the lack of caching here and below for hte client. In AWSs IO we cache the client whereas here we create a new one every time. That could be very expensive

Caching can be implemented on the client side with something like () -> someClient

Where someClient is is the same throughout the entire lifecycle of AdlsFileIO.

If you look at how this class used to work, it created a new client every time that we call client

https://github.com/sarthaksin1857/iceberg/blob/main/azure/src/main/java/org/apache/iceberg/azure/adlsv2/ADLSFileIO.java#L112-L121

IMO it makes sense to leave it as is and leave caching to the client, WDYT?

Its cached in the other FileIOs though. Its not clear from the API contract that the user is responsible for the caching and its different from the S3FileIO in that regard.

It seems like all the classes which have supplier have caching so it makes sense to add it here. I pushed a commit which caches just the supplier client and returns it on subsequent gets.

I did not add caching to the existing code and I did not follow the same map approach used in the other fileIo classes.

Reasoning for this is scope creep.

kevinjqliu

thanks for adding this, left a few comments

kevinjqliu · 2026-01-07T19:19:41Z

azure/src/main/java/org/apache/iceberg/azure/adlsv2/ADLSFileIO.java

-        new DataLakeFileSystemClientBuilder().httpClient(HTTP);
+    if (clientSupplier != null) {
+      return clientSupplier.get();
+    } else {


+1 to removing else, and from the constructor above.
lets keep one place where the clientSupplier is used since everything else calls this method

kevinjqliu · 2026-01-07T19:21:15Z

azure/src/main/java/org/apache/iceberg/azure/adlsv2/ADLSFileIO.java

+   * <p>Calling {@link ADLSFileIO#initialize(Map)} will overwrite the properties and azureProperties
+   * set in this constructor, but the clientSupplier will continue to be used.


nit: not sure if we need all this explanation since both properties and azureProperties are just initialized with empty value

rymurr · 2026-01-20T10:34:27Z

azure/src/main/java/org/apache/iceberg/azure/adlsv2/ADLSFileIO.java

+   */
+  public ADLSFileIO(SerializableSupplier<DataLakeFileSystemClient> clientSupplier) {
+    this.clientSupplier = clientSupplier;
+    this.properties = SerializableMap.copyOf(Maps.newHashMap());


if that is the case would it be better to call initialize from the constructor. To make the contract more clear and ensure if we change initialize we don't have to change the constructo

rymurr · 2026-01-20T11:32:28Z

azure/src/main/java/org/apache/iceberg/azure/adlsv2/ADLSFileIO.java


  @VisibleForTesting
  DataLakeFileSystemClient client(ADLSLocation location) {
+    if (clientSupplier != null) {


this means you can't use the client with multiple locations now. I think you have to handle a single client per location

The main difference here an aws/gcp is that one client can be used for multiple locations/buckets. In azure the client is tied to a certain container/path. I will explore creating a client with A Function<Location, ADLSClient> so that we can specify multiple containers with this class

spotless Add tests/cleanup remove redundant null check remove redundant comment Add caching just for supplier method Cache ADLS clients and call initialize in constructor use host for key caching

github-actions bot added the AZURE label Jan 5, 2026

sarthaksin1857 marked this pull request as ready for review January 5, 2026 18:13

rymurr reviewed Jan 7, 2026

View reviewed changes

kevinjqliu reviewed Jan 7, 2026

View reviewed changes

rymurr reviewed Jan 20, 2026

View reviewed changes

Add DataLakeFileSystemClient constructor in ADLSFileIO

ead6304

spotless Add tests/cleanup remove redundant null check remove redundant comment Add caching just for supplier method Cache ADLS clients and call initialize in constructor use host for key caching

sarthaksin1857 force-pushed the add-adls-supplier-fileIo branch from 0751289 to ead6304 Compare January 23, 2026 15:53

		* <p>Calling {@link ADLSFileIO#initialize(Map)} will overwrite the properties and azureProperties
		* set in this constructor, but the clientSupplier will continue to be used.

Add DataLakeFileSystemClient constructor in ADLSFileIO #14966

Are you sure you want to change the base?

Add DataLakeFileSystemClient constructor in ADLSFileIO #14966

Conversation

sarthaksin1857 commented Jan 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Reasoning

Uh oh!

sarthaksin1857 commented Jan 5, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sarthaksin1857 Jan 7, 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sarthaksin1857 Jan 7, 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!

sarthaksin1857 Jan 7, 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!

kevinjqliu left a comment

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!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

sarthaksin1857 commented Jan 5, 2026 •

edited

Loading

sarthaksin1857 Jan 7, 2026 •

edited

Loading

sarthaksin1857 Jan 7, 2026 •

edited

Loading

sarthaksin1857 Jan 7, 2026 •

edited

Loading