Skip to content

Remove sync credentials #551

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jamesmyatt wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Remove sync credentials #551

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bf2887c
Remove sync_credentials
jamesmyatt Jun 15, 2026
b031953
Fix blank lines
jamesmyatt Jun 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 13 additions & 37 deletions adlfs/spec.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -362,12 +362,7 @@ def __init__(
and self.client_id is not None
and self.connection_string is None
):
(
self.credential,
self.sync_credential,

@kyleknap kyleknap Jun 15, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we add a changelog entry noting that we removed the sync_credential property as it was unused and label it as **Breaking** similar to other entries?

) = self._get_credential_from_service_principal()
else:
self.sync_credential = None
self.credential = self._get_credential_from_service_principal()

# Solving issue in https://github.com/fsspec/adlfs/issues/270
if (
Expand All @@ -377,10 +372,7 @@ def __init__(
and self.account_key is None
and self.connection_string is None
):
(
self.credential,
self.sync_credential,
) = self._get_default_azure_credential(**kwargs)
self.credential = self._get_default_azure_credential(**kwargs)

self.do_connect()
weakref.finalize(self, sync, self.loop, close_service_client, self)
Expand Down Expand Up @@ -469,50 +461,34 @@ def _get_kwargs_from_urls(urlpath):

def _get_credential_from_service_principal(self):
"""
Create a Credential for authentication. This can include a TokenCredential
client_id, client_secret and tenant_id
Create a Credential for authentication.

This can include a TokenCredential client_id, client_secret and tenant_id.

Returns
-------
Tuple of (Async Credential, Sync Credential).
Async Credential
"""
from azure.identity import ClientSecretCredential
from azure.identity.aio import (
ClientSecretCredential as AIOClientSecretCredential,
)
from azure.identity.aio import ClientSecretCredential

async_credential = AIOClientSecretCredential(
return ClientSecretCredential(
tenant_id=self.tenant_id,
client_id=self.client_id,
client_secret=self.client_secret,
)

sync_credential = ClientSecretCredential(
tenant_id=self.tenant_id,
client_id=self.client_id,
client_secret=self.client_secret,
)

return (async_credential, sync_credential)

def _get_default_azure_credential(self, **kwargs):
"""
Create a Credential for authentication using DefaultAzureCredential
Create a Credential for authentication using DefaultAzureCredential.

Returns
-------
Tuple of (Async Credential, Sync Credential).
Async Credential
"""

from azure.identity import DefaultAzureCredential
from azure.identity.aio import (
DefaultAzureCredential as AIODefaultAzureCredential,
)

async_credential = AIODefaultAzureCredential(**kwargs)
sync_credential = DefaultAzureCredential(**kwargs)
from azure.identity.aio import DefaultAzureCredential

return (async_credential, sync_credential)
return DefaultAzureCredential(**kwargs)

def do_connect(self):
"""Connect to the BlobServiceClient, using user-specified connection details.
Expand Down Expand Up @@ -2140,7 +2116,7 @@ def connect_client(self):
f"https://{self.fs.account_name}.blob.core.windows.net"
)

creds = [self.fs.sync_credential, self.fs.account_key, self.fs.credential]
creds = [self.fs.credential, self.fs.account_key]

@kyleknap kyleknap Jun 15, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not actionable but just calling this out... Technically this is a change of behavior as before if a credential was provided to the file system a sync_credential would not be set and account_key would thus take precedence over the explicitly provided credential. That being said, this update seems reasonable as:

  1. It make it consistent with how the filesystem class resolves credentials (e.g. it prioritizes self.credential over self.account_key no matter what)
  2. In most cases, the container client will already be instantiated from the file system so this code path in general should not be hit unless connect_client is explicitly called.

if any(creds):
self.container_client = [
_create_aio_blob_service_client(
Expand Down
Loading