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

fix: no timeout for urlopen issue #526 #527

Merged
merged 1 commit into from
Jan 22, 2024
Merged

fix: no timeout for urlopen issue #526 #527

merged 1 commit into from
Jan 22, 2024

Conversation

grzracz
Copy link
Contributor

@grzracz grzracz commented Jan 20, 2024

No description provided.

@CLAassistant
Copy link

CLAassistant commented Jan 20, 2024

CLA assistant check
All committers have signed the CLA.

Copy link
Contributor

@jannotti jannotti left a comment

Choose a reason for hiding this comment

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

urlopen docs say: The optional timeout parameter specifies a timeout in seconds for blocking operations like the connection attempt (if not specified, the global default timeout setting will be used). Any idea what that global is?

Regardless, it seems right to me to let the caller have access to the setting. I'd like @jasonpaulos to weigh in.

Another possible is to allow arbitrary **kwargs, and pass them through, if there are other useful features of urlopen that it would give access to. It doesn't seem like it though, the rest appears to about SSL details.

algosdk/kmd.py Show resolved Hide resolved
algosdk/v2client/algod.py Show resolved Hide resolved
@grzracz
Copy link
Contributor Author

grzracz commented Jan 20, 2024

Seems like the default timeout is implementation-dependent and in Python it just never times out:
https://stackoverflow.com/questions/29649173/what-is-the-global-default-timeout

@jasonpaulos jasonpaulos linked an issue Jan 22, 2024 that may be closed by this pull request
Copy link
Contributor

@jasonpaulos jasonpaulos left a comment

Choose a reason for hiding this comment

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

This looks like a good improvement. Thanks for the contribution

@jasonpaulos jasonpaulos merged commit 12d5fa8 into algorand:develop Jan 22, 2024
12 of 13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

algod_request method hangs forever on network change
4 participants