Skip to content
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.

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

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

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

Conversation

gabrieldaiha
Copy link
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
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
Copy link
Collaborator

hayesgb commented Jul 25, 2022

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

@gabrieldaiha
Copy link
Contributor Author

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

Any news?

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.

2 participants