Fix issues #618
Fix issues #618
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #618 +/- ##
==========================================
Coverage ? 89.76%
==========================================
Files ? 32
Lines ? 3077
Branches ? 0
==========================================
Hits ? 2762
Misses ? 315
Partials ? 0
Continue to review full report at Codecov.
|
openml/_api_calls.py
Outdated
| return response.text | ||
|
|
||
|
|
||
| def send_request(request_method, url, kwargs): |
There was a problem hiding this comment.
i hate kwargs, it's so vague. Do we need this level of flexibility or can we just add optional arguments?
|
|
||
|
|
||
| def _perform_api_call(call, data=None, file_elements=None, | ||
| add_authentication=True): |
There was a problem hiding this comment.
don't you need the authentication flag? GET requests don't require authentication, POST/DELETE requests do.
There was a problem hiding this comment.
That is handled internally and does not need to be an argument to the openml-api call API.
|
|
||
| # Increase the number of retries to avoid spurios server failures | ||
| self.connection_n_retries = openml.config.connection_n_retries | ||
| openml.config.connection_n_retries = 10 |
There was a problem hiding this comment.
I'm a bit confused about the number of retries being stored in the _default dict, the openml.config file and self.connection_n_retries. So, by default it is 2, then when that doesn't work, it is increased to 10 and stored in the openml.config, and then it stops (it doesn't get higher than 10)?
There was a problem hiding this comment.
Nope, it's increased to 10 for testing in every case. It is stored in the class to restore the original value afterwards.
There was a problem hiding this comment.
I agree that it is unfortunate that the value of 2 is twice in the original file, I just made a note to change it later.
| if i == n_retries: | ||
| raise e | ||
| else: | ||
| time.sleep(0.1 * i) |
Fixes jamming the server with too many concurrent requests by adding a new flag.