Skip to content
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

Extend DiscoveryBase with api_factory setting #73

Merged
merged 3 commits into from
Dec 1, 2021
Merged

Extend DiscoveryBase with api_factory setting #73

merged 3 commits into from
Dec 1, 2021

Conversation

tony-romanovych
Copy link
Contributor

Currently, there's no simple way to adjust internal API client behavior while using discovery at the same time. To do that, one has to somehow override DiscoveryBase._configure_api_client, that is creating new API instances (e.g. BasicApi, BatchApi).

This PR adds optional config setting - api_factory. If present, it's expected to be a callable constructing new API instances. It will be used instead of default instantiation logic. These changes are not breaking the public interface of Discovery classes or Client.

Why:

My primary goal was to make API exception handling more clean, because:
a) There's a bunch of generated ApiException classes: if I work with multiple crm objects, I have to import all those exception classes, give them aliases, and I cannot catch them with single except ApiException
b) There's no way currently to specifically handle only 404 with a single row, e.g. except NotFoundException.
This syntax can be achieved though by extending ApiClient.

from hubspot.crm.deals.exceptions import ApiException as DealsApiException
from hubspot.crm.contacts.exceptions import ApiException as ContactsApiException

try:
    call_hubspot_api()
except DealsApiException as e:
    if e.status == 404:
        ...
    else:
        raise

VERSION Outdated Show resolved Hide resolved
@plaurynovich-hubspot plaurynovich-hubspot merged commit 5ab761d into HubSpot:master Dec 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants