Skip to content

Fixed weakref async credential close when self.credential is None#330

Closed
gabrieldaiha wants to merge 16 commits into
fsspec:mainfrom
gabrieldaiha:master
Closed

Fixed weakref async credential close when self.credential is None#330
gabrieldaiha wants to merge 16 commits into
fsspec:mainfrom
gabrieldaiha:master

Conversation

@gabrieldaiha

Copy link
Copy Markdown
Contributor

The PR #328 included a weakref to close async credential, to avoid the Unclosed Client Session warning.

This PR aims to fix the case that we can have self.credential = None after passing the self.do_connect.

Lines 470-471 of spec.py is the fix:
https://github.com/gabrieldaiha/adlfs/blob/d2a5be68af7934025f884d2dc523f1708738c6d7/adlfs/spec.py#L470-L471

@gabrieldaiha

Copy link
Copy Markdown
Contributor Author

Tests are claiming an error about resource name size that it does not have relationship with this PR.
@hayesgb anything happened with yesterday's MR in these tests?

  • test_fetch_entire_blob
  • test_fetch_first_half
  • test_fetch_middle

@hayesgb

hayesgb commented Jul 25, 2022

Copy link
Copy Markdown
Collaborator

Not that I'm aware of, but I'll check. Its interesting that its only failing in DEV

@gabrieldaiha

Copy link
Copy Markdown
Contributor Author

Not that I'm aware of, but I'll check. Its interesting that its only failing in DEV

Any news?

@kyleknap

kyleknap commented May 5, 2026

Copy link
Copy Markdown
Collaborator

This looks now handled based on this code path:

adlfs/adlfs/spec.py

Lines 381 to 382 in 74ed2be

if self.credential is not None:
weakref.finalize(self, sync, self.loop, close_credential, self)
Closing PR.

@kyleknap kyleknap closed this May 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants