Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions doc/progress.rst
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ Changelog

0.12.2
~~~~~~
* ADD #1065: Add a ``retry_policy`` configuration option that determines the frequency and number of times to attempt to retry server requests.
* ADD #1075: A docker image is now automatically built on a push to develop. It can be used to build docs or run tests in an isolated environment.
* DOC: Fixes a few broken links in the documentation.
* MAINT/DOC: Automatically check for broken external links when building the documentation.
Expand Down
9 changes: 7 additions & 2 deletions doc/usage.rst
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,14 @@ which are separated by newlines. The following keys are defined:
* if set to ``True``, when ``run_flow_on_task`` or similar methods are called a lookup is performed to see if there already exists such a run on the server. If so, download those results instead.
* if not given, will default to ``True``.

* retry_policy:
* Defines how to react when the server is unavailable or experiencing high load. It determines both how often to attempt to reconnect and how quickly to do so. Please don't use ``human`` in an automated script that you run more than one instance of, it might increase the time to complete your jobs and that of others.
* human (default): For people running openml in interactive fashion. Try only a few times, but in quick succession.
* robot: For people using openml in an automated fashion. Keep trying to reconnect for a longer time, quickly increasing the time between retries.

* connection_n_retries:
* number of connection retries.
* default: 2. Maximum number of retries: 20.
* number of connection retries
* default depends on retry_policy (5 for ``human``, 50 for ``robot``)

* verbosity:
* 0: normal output
Expand Down
16 changes: 14 additions & 2 deletions openml/_api_calls.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@
import time
import hashlib
import logging
import math
import pathlib
import random
import requests
import urllib.parse
import xml
Expand Down Expand Up @@ -217,7 +219,7 @@ def __is_checksum_equal(downloaded_file, md5_checksum=None):


def _send_request(request_method, url, data, files=None, md5_checksum=None):
n_retries = max(1, min(config.connection_n_retries, config.max_retries))
n_retries = max(1, config.connection_n_retries)

response = None
with requests.Session() as session:
Expand Down Expand Up @@ -261,7 +263,17 @@ def _send_request(request_method, url, data, files=None, md5_checksum=None):
if retry_counter >= n_retries:
raise
else:
time.sleep(retry_counter)

def robot(n: int) -> float:
wait = (1 / (1 + math.exp(-(n * 0.5 - 4)))) * 60
variation = random.gauss(0, wait / 10)
return max(1.0, wait + variation)

def human(n: int) -> float:
return max(1.0, n)

delay = {"human": human, "robot": robot}[config.retry_policy](retry_counter)
time.sleep(delay)
if response is None:
raise ValueError("This should never happen!")
return response
Expand Down
40 changes: 34 additions & 6 deletions openml/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -149,19 +149,17 @@ def check_cache_dir(path: str) -> str:
def configure_connection_n_retries(value: str) -> None:
def valid_connection_retries(n: str) -> str:
if not n.isdigit():
return f"Must be an integer number (smaller than {config.max_retries})."
if int(n) > config.max_retries:
return f"connection_n_retries may not exceed {config.max_retries}."
if int(n) == 0:
return "connection_n_retries must be non-zero."
return f"'{n}' is not a valid positive integer."
if int(n) <= 0:
return "connection_n_retries must be positive."
return ""

configure_field(
field="connection_n_retries",
value=value,
check_with_message=valid_connection_retries,
intro_message="Configuring the number of times to attempt to connect to the OpenML Server",
input_message=f"Enter an integer between 0 and {config.max_retries}: ",
input_message="Enter a positive integer: ",
)


Expand Down Expand Up @@ -217,6 +215,35 @@ def is_zero_through_two(verbosity: str) -> str:
)


def configure_retry_policy(value: str) -> None:
def is_known_policy(policy: str) -> str:
if policy in ["human", "robot"]:
return ""
return "Must be 'human' or 'robot'."

def autocomplete_policy(policy: str) -> str:
for option in ["human", "robot"]:
if option.startswith(policy.lower()):
return option
return policy

intro_message = (
"Set the retry policy which determines how to react if the server is unresponsive."
"We recommend 'human' for interactive usage and 'robot' for scripts."
"'human': try a few times in quick succession, less reliable but quicker response."
"'robot': try many times with increasing intervals, more reliable but slower response."
)

configure_field(
field="retry_policy",
value=value,
check_with_message=is_known_policy,
intro_message=intro_message,
input_message="Enter 'human' or 'robot': ",
sanitize=autocomplete_policy,
)


def configure_field(
field: str,
value: Union[None, str],
Expand Down Expand Up @@ -272,6 +299,7 @@ def configure(args: argparse.Namespace):
"apikey": configure_apikey,
"server": configure_server,
"cachedir": configure_cachedir,
"retry_policy": configure_retry_policy,
"connection_n_retries": configure_connection_n_retries,
"avoid_duplicate_runs": configure_avoid_duplicate_runs,
"verbosity": configure_verbosity,
Expand Down
46 changes: 29 additions & 17 deletions openml/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import os
from pathlib import Path
import platform
from typing import Tuple, cast, Any
from typing import Tuple, cast, Any, Optional
import warnings

from io import StringIO
Expand Down Expand Up @@ -95,11 +95,10 @@ def set_file_log_level(file_output_level: int):
else os.path.join("~", ".openml")
),
"avoid_duplicate_runs": "True",
"connection_n_retries": "10",
"max_retries": "20",
"retry_policy": "human",
"connection_n_retries": "5",
}


# Default values are actually added here in the _setup() function which is
# called at the end of this module
server = str(_defaults["server"]) # so mypy knows it is a string
Expand All @@ -122,9 +121,26 @@ def get_server_base_url() -> str:
cache_directory = str(_defaults["cachedir"]) # so mypy knows it is a string
avoid_duplicate_runs = True if _defaults["avoid_duplicate_runs"] == "True" else False

# Number of retries if the connection breaks
retry_policy = _defaults["retry_policy"]
connection_n_retries = int(_defaults["connection_n_retries"])
max_retries = int(_defaults["max_retries"])


def set_retry_policy(value: str, n_retries: Optional[int] = None) -> None:
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Due to the connection_n_retries and retry_policy being linked, I added a function to change them together.
I think we'll want to rework all configuration properties at some point anyway to only set them by functions.
The function could then also optionally write the new setting directly to the configuration file, and should execute checks to see if it's set to a valid value (this would in turn simplify the cli code).

global retry_policy
global connection_n_retries
default_retries_by_policy = dict(human=5, robot=50)

if value not in default_retries_by_policy:
raise ValueError(
f"Detected retry_policy '{value}' but must be one of {default_retries_by_policy}"
)
if n_retries is not None and not isinstance(n_retries, int):
raise TypeError(f"`n_retries` must be of type `int` or `None` but is `{type(n_retries)}`.")
if isinstance(n_retries, int) and n_retries < 1:
raise ValueError(f"`n_retries` is '{n_retries}' but must be positive.")

retry_policy = value
connection_n_retries = default_retries_by_policy[value] if n_retries is None else n_retries


class ConfigurationForExamples:
Expand Down Expand Up @@ -205,8 +221,6 @@ def _setup(config=None):
global server
global cache_directory
global avoid_duplicate_runs
global connection_n_retries
global max_retries

config_file = determine_config_file_path()
config_dir = config_file.parent
Expand Down Expand Up @@ -238,8 +252,12 @@ def _get(config, key):
apikey = _get(config, "apikey")
server = _get(config, "server")
short_cache_dir = _get(config, "cachedir")
connection_n_retries = int(_get(config, "connection_n_retries"))
max_retries = int(_get(config, "max_retries"))

n_retries = _get(config, "connection_n_retries")
if n_retries is not None:
n_retries = int(n_retries)

set_retry_policy(_get(config, "retry_policy"), n_retries)

cache_directory = os.path.expanduser(short_cache_dir)
# create the cache subdirectory
Expand All @@ -261,12 +279,6 @@ def _get(config, key):
"not working properly." % config_dir
)

if connection_n_retries > max_retries:
raise ValueError(
"A higher number of retries than {} is not allowed to keep the "
"server load reasonable".format(max_retries)
)


def set_field_in_config_file(field: str, value: Any):
""" Overwrites the `field` in the configuration file with the new `value`. """
Expand Down Expand Up @@ -317,7 +329,7 @@ def get_config_as_dict():
config["cachedir"] = cache_directory
config["avoid_duplicate_runs"] = avoid_duplicate_runs
config["connection_n_retries"] = connection_n_retries
config["max_retries"] = max_retries
config["retry_policy"] = retry_policy
return config


Expand Down
4 changes: 3 additions & 1 deletion openml/testing.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,9 @@ def setUp(self, n_levels: int = 1):
openml.config.cache_directory = self.workdir

# Increase the number of retries to avoid spurious server failures
self.retry_policy = openml.config.retry_policy
self.connection_n_retries = openml.config.connection_n_retries
openml.config.connection_n_retries = 10
openml.config.set_retry_policy("robot", n_retries=20)

def tearDown(self):
os.chdir(self.cwd)
Expand All @@ -109,6 +110,7 @@ def tearDown(self):
raise
openml.config.server = self.production_server
openml.config.connection_n_retries = self.connection_n_retries
openml.config.retry_policy = self.retry_policy

@classmethod
def _mark_entity_for_removal(self, entity_type, entity_id):
Expand Down
5 changes: 5 additions & 0 deletions tests/test_datasets/test_dataset_functions.py
Original file line number Diff line number Diff line change
Expand Up @@ -506,6 +506,9 @@ def test__getarff_md5_issue(self):
"oml:md5_checksum": "abc",
"oml:url": "https://www.openml.org/data/download/61",
}
n = openml.config.connection_n_retries
openml.config.connection_n_retries = 1

self.assertRaisesRegex(
OpenMLHashException,
"Checksum of downloaded file is unequal to the expected checksum abc when downloading "
Expand All @@ -514,6 +517,8 @@ def test__getarff_md5_issue(self):
description,
)

openml.config.connection_n_retries = n

def test__get_dataset_features(self):
features_file = _get_dataset_features_file(self.workdir, 2)
self.assertIsInstance(features_file, str)
Expand Down
2 changes: 1 addition & 1 deletion tests/test_openml/test_api_calls.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,4 +29,4 @@ def test_retry_on_database_error(self, Session_class_mock, _):
):
openml._api_calls._send_request("get", "/abc", {})

self.assertEqual(Session_class_mock.return_value.__enter__.return_value.get.call_count, 10)
self.assertEqual(Session_class_mock.return_value.__enter__.return_value.get.call_count, 20)
6 changes: 3 additions & 3 deletions tests/test_openml/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ def test_get_config_as_dict(self):
_config["server"] = "https://test.openml.org/api/v1/xml"
_config["cachedir"] = self.workdir
_config["avoid_duplicate_runs"] = False
_config["connection_n_retries"] = 10
_config["max_retries"] = 20
_config["connection_n_retries"] = 20
_config["retry_policy"] = "robot"
self.assertIsInstance(config, dict)
self.assertEqual(len(config), 6)
self.assertDictEqual(config, _config)
Expand All @@ -57,8 +57,8 @@ def test_setup_with_config(self):
_config["server"] = "https://www.openml.org/api/v1/xml"
_config["cachedir"] = self.workdir
_config["avoid_duplicate_runs"] = True
_config["retry_policy"] = "human"
_config["connection_n_retries"] = 100
_config["max_retries"] = 1000
orig_config = openml.config.get_config_as_dict()
openml.config._setup(_config)
updated_config = openml.config.get_config_as_dict()
Expand Down