I would move the settings to the individual classes. I think this design introduces too high coupling of the classes to this file. You cannot move the classes around, or add a new API version without making non-extensible changes to this file here - because APISettings will require a constructor change and new classes it accepts.

Instead, a better design is to apply the strategy pattern cleanly to the different API definitions - v1 and v2 - and move the config either to their __init__, or a set_config (or similar) method.

I have updated the Config class, which can handle multiple api-versions in future. Constructor change can still be expected not for api-versions but for new config values, let me know if that is still problematic

Please rename the parameters. The description is incomplete though more accurate than the current name reset_cache: the cache is bypassed on load, not reset. The current cache is unaffected, which is unexpected to me for a parameter "reset_cache".

I would find it much more intuitive if the parameters are store_to_cache and load_from_cache, to indicate if a response will be attempted to be saved to or loaded from the cache.

I agree that the naming and description is not very clean, I will update. How about this:

enable_cache : bool, optional
    Whether to load/store response from cache.
refresh_cache : bool, optional
    If True, bypass existing cache and over-write with new response.
    This parameter is ignored if `enable_cache=False`.

And the internal implementation stays the same. This fulfills the sdk requirements for cache handling.

I still think store_to_cache and load_from_cache are not well suited for the sdk, that shifts some logic handling to the sdk, where as here the logic stays with the client and sdk only commands for a specific behaviour.

Any reason for using StrEnum?

I am not using StrEnum, are you asking why not?

I switched to this only because of the mypy errors in pre-commit checks.

from enum import StrEnum
# error: Module "enum" has no attribute "StrEnum"  [attr-defined]

api_version: APIVersion = APIVersion.V1
# incompatible type "str"; expected "APIVersion"

I can ignore the [attr-defined] error and fix type hints to str, but that didnot feel good to have str in type hints instead of the enum.

The singleton implementation in get_instance is not thread-safe. In a multi-threaded environment, multiple threads could simultaneously check cls._instance is None and create multiple instances. Consider using a threading lock or the @functools.lru_cache decorator with maxsize=1 to ensure thread-safe singleton behavior.

Line 73 returns Path(xdg_cache_home) instead of cache_dir when an old cache directory is found. This is inconsistent with the warning message which says files will be located in cache_dir (line 71), but the function actually returns the parent directory. This should likely return cache_dir to match the documented behavior.

The docstring for ResourceAPI.__init__ states that the minio parameter is "optional" (line 29), but the method signature on line 47 requires a MinIOClient instance (no None allowed). Either update the signature to accept MinIOClient | None to match the documentation, or remove "optional" from the docstring. The attribute type on line 40 already suggests it could be None.

The __all__ list contains duplicate entries: "FallbackProxy" appears on lines 68 and 69, and "ResourceAPI" appears on lines 76 and 77. Remove the duplicate entries to maintain clean exports.

Line 48 returns Path("~", ".cache", "openml") without calling .expanduser(). This means the tilde character will be treated literally rather than being expanded to the user's home directory. Add .expanduser() to ensure proper path expansion, like: return Path("~", ".cache", "openml").expanduser().

The methods publish, delete, tag, and untag in ResourceV2API have return type annotations (int, bool, list[str]) but they call _not_supported which always raises an exception and never returns. These methods should have NoReturn as their return type annotation to accurately reflect that they always raise an exception.

The HTTPCache class docstring mentions "Entries are considered valid until their time-to-live (TTL) expires" (line 39-40), and the load method's docstring states it may raise TimeoutError if "the cached entry has expired based on the configured TTL" (line 119). However, there's no TTL implementation in the code - the created_at timestamp is loaded (line 138) but never checked against any TTL value. Either implement TTL checking or remove the misleading documentation about TTL expiration.

The _robot_delay and _human_delay methods in the HTTPClient class are never used. Line 242 uses human_delay and robot_delay functions imported from utils module instead. Remove these unused methods to avoid confusion and reduce code duplication, or update line 242 to use these instance methods if they were intended to be used.

The _find_attr method returns the first attribute that is not None, which could cause issues if an API implementation legitimately has an attribute set to None. Consider using hasattr instead, or checking if the attribute exists in the object's dictionary rather than checking if its value is not None.

Why not ==?

The functions in this file are duplicate from the functions in http.py. Please de-duplicate (I would suggest removing this file).

In practice, the api_key can be None. For example, get requests do not require it, and some tests explicitly set it to None. I think this should be changed.

Following up on that, we should probably add a test to ensure we raise an exception when use_api_key=True and api_key=None.

I don't understand why we need this in addition to the global config? Is it so that different API versions can have different URLs? If this is the case, should different API versions also have different cache directories, or is the API version automatically part of the URL, and thereby, part of the cache directory?

Why is this a separate class?

Don't we have an enum that maps from their Python equivalent to the actual name? Shouldn't we use that here?

Same as above.

Same as above.

why do we need a separate variable for this?

Why do we need a separate variable for this?