-
-
Notifications
You must be signed in to change notification settings - Fork 25.2k
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 OpenML timeout #23358
Fix OpenML timeout #23358
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM as well. I see no reason why the retry delay should be of the same order as a network timeout. The only purpose of the retry delay is to avoid DOSing a service that is already potentially overloaded via a retry mechanism.
This was probably overlooked during the initial review of the PR that introduced the retry mechanism.
Fix #23357.
I am not sure why we were passing
timeout=delay
and I did not find anything in #21901. They are not really the same thing,delay
is the time to wait for between two attempts,timeout
is the time to wait to get the data.By not passing timeout, we use the default timeout which is "no timeout" (i.e. "wait for ever") by default. You can always use
socket.setdefaulttimeout
if you want to change it.Side-comment: maybe part of the reason we did not see this before is because OpenML started to do some redirections recently (openml.org -> old.openml.org) and for some datasets, that just happens to go over the timeout of 1s we were using. The timeout
_get_data_info_by_name
in the issue OP and in_get_data_features
in my case.