unioslo / harborapi

Python async client for the Harbor REST API v2.0.
https://unioslo.github.io/harborapi/
MIT License
28 stars 5 forks source link

Rename `HarborAsyncClient` to `HarborClient` #8

Open pederhan opened 1 year ago

pederhan commented 1 year ago

harborapi is first and foremost and async package, and as such the main client implementation shouldn't need "Async" tacked onto its name. Having the names HarborAsyncClient and HarborSyncClient is a bit of an eyesore, and it would be better to have HarborClient and HarborSyncClient to denote that the async client is the primary implementation and that the sync client is a modification of the original async client.

In order to not break the installations of current users (all 0 of them), the harborapi and harborapi.client modules should provide HarborClient AND HarborAsyncClient, but with a deprecation warning issued for the latter.

We will need to add some sort of import hook, or perhaps create HarborAsyncClient by subclassing HarborClient and issuing a warning in the constructor:

import warnings

class HarborAsyncClient(HarborClient):
    def __init__(self, *args, **kwargs) -> None:
        super().__init__(*args, **kwargs)
        warnings.warn("HarborAsyncClient is deprecated and will be removed, use HarborClient instead.", DeprecationWarning)

This will of course break IDE auto-completion for the __init__ function of HarborAsyncClient, but that fact combined with the warning should make people switch over to HarborClient.

pederhan commented 1 year ago

Instead of subclassing, we could probably just modify the __new__ method of this hypothetical HarborAsyncClient to return HarborClient instead, so no matter which one you try to instantiate, you always get the same type.

I mean, yeah, a subclassed HarborAsyncClient would pass any isinstance checks for HarborClient but, it's probably better to not introduce a whole new type that can mess things up.