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

http: only close connector if it is available #779

Merged
merged 2 commits into from
Sep 30, 2021

Conversation

isidentical
Copy link
Member

Stuff like asyncio-retry offer their own clients which proxy the original ClientSession, and they do not bind stuff like _connector directly. Since this is a private-attribute, it might be nicer if we access it optionally and only try to close if we can find it.

@martindurant
Copy link
Member

For py 3.6, I think you need loop.run_until_complete

@isidentical isidentical force-pushed the dont-access-gone-connector branch from 97d5dda to 2486401 Compare September 29, 2021 19:13
@isidentical
Copy link
Member Author

@martindurant
Copy link
Member

That's another solution :)

@martindurant
Copy link
Member

I am running the tests again, but the windows async failure is a little worrying, even if it was a one-off.

@isidentical
Copy link
Member Author

I recall seeing this error before too. Would you mind explaining what test_async_other_thread does?

@martindurant
Copy link
Member

what test_async_other_thread does

The filesystem's loop is running in another explicit thread, and we manipulate the loop object directly to get it to do things (i.e., not using async/await). This is actually quite similar in effect to specifying no loop/asyncronous flags at all and calling the blocking functions.

@martindurant
Copy link
Member

Perhaps try changing

loop = asyncio.get_event_loop()
to
loop = asyncio.new_event_loop()

(don't know why this should show up just now...)

@isidentical
Copy link
Member Author

isidentical commented Sep 30, 2021

The only thing that I guess cause this to show up now is the order of tests, let me temporarily move the test to the end and see if it passes or not.

@martindurant
Copy link
Member

OK, well it passed - we can revisit if it happens again.

@martindurant martindurant merged commit 2fc331f into fsspec:master Sep 30, 2021
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