feat(rest_api): custom client for specific resources #2082

joscha · 2024-11-21T02:01:03Z

Description

This adds the ability to pass a custom ClientConfig to an EndpointResource.
The passed ClientConfig object will be merged with the global one from the containing @dlt.resource.

Example

{
    "client": {
        "base_url": "https://api.mything.com",
        # Default auth is using Bearer authentication
        "auth": {
            "type": "bearer",
            "token": dlt.secrets["your_api_token"],
        }
    },
    "resources": [
        "resource-using-bearer-auth", # <-- for example this resource
        {
            "name": "my-resource-with-special-auth",
            "client": {
                # However this resource is using HTTP Basic auth instead.
                "auth": HttpBasicAuth("user", dlt.secrets["your_basic_auth_password"]) 
            },
            # ...
        }
    ]
    # ...
}

In the corresponding Slack thread a question came up whether a custom authenticator could be used. The answer is yes, it would be possible to write custom authenticator which introspects the resource requested (base on the URL for example), however the implementation of such an authenticator would be a lot more involved, less obvious than just merging a configuration object and it would also not have an answer for differing base_urls , headers or other client-specific parameters for example.

Related Issues

Fixes #...
Closes #...
Resolves #...

Additional Context

See originating Slack thread.

The reason for this feature is that some API endpoints, even though available under the same base URL and returning the same entities, may have different ways of accessing them. For example the Affinity V2 API uses an OpenAPI-based access model with Bearer authentication. This part of the API is available under https://api.affinity.co/v2/.
The first version of the API is available under https://api.affinity.co/ but uses HTTP Basic Auth.
Entities from both APIs are using the same entity IDs and are related, thus must be specified in the same @dlt.resource in order to define those relations.
This is currently (1.4.0) not possible, due to the differing (incompatible) authentication methods.

joscha · 2024-11-21T02:01:56Z

dlt/sources/rest_api/__init__.py

@@ -263,12 +263,17 @@ def create_resources(
            incremental_cursor_transform,
        ) = setup_incremental_object(request_params, endpoint_config.get("incremental"))

+        merged_client_config: ClientConfig = {


configs get merged here, with the keys defined in the endpoint taking preference over the global one.

netlify · 2024-11-21T02:01:58Z

✅ Deploy Preview for dlt-hub-docs canceled.

Name	Link
🔨 Latest commit	`b80734e`
🔍 Latest deploy log	https://app.netlify.com/sites/dlt-hub-docs/deploys/6746e64f9da64d00080590de

joscha · 2024-11-21T02:04:17Z

dlt/sources/rest_api/typing.py

    headers: Optional[Dict[str, str]]
    auth: Optional[AuthConfig]
    paginator: Optional[PaginatorConfig]
    session: Optional[Session]


+class ClientConfig(BaseClientConfig, total=False):
+    base_url: str  # type: ignore[misc]


Typing overrides are currently not yet supported (see python/mypy#7435) thus the ignore here.
For the optional dict in each endpoint we need to ensure that all keys are optional.

tests/sources/rest_api/configurations/source_configs.py

dlt/sources/rest_api/__init__.py

francescomucio · 2024-11-21T14:45:12Z

dlt/sources/rest_api/__init__.py

+    resources = c.get("resources")
+    if resources:
+        for resource in resources:
+            if isinstance(resource, str) or isinstance(resource, DltResource):


I would go for the opposite check (is instance of EndpointResource?), is there a reason for testing like this

Yes, it's a TypedDict and the check is not (yet) supported on it :-/

sure, you could test only for dict, but that should be enough. if's it's a dict (even if an incorrect one) the code will try to mask the client field. Then the real validation will happen.

It is not super precise (a comment to mention that the actual test should be for the proper type, but python...), but a bit simpler to read.

Happy to hear other opinions @burnash

I am happy to swap it for a check for dict if that's preferred. Whilst not amazing to read, the current type is a Union out of three possible types, so it currently is precise, but not super future proof.

dlt/sources/rest_api/__init__.py

joscha · 2024-11-22T10:28:47Z

@willi-mueller @burnash would you mind taking a look and give me feedback on this, please?

burnash · 2024-11-24T22:23:50Z

Hi @joscha, thanks you for the contribution. I think this is a great idea. As you may have noticed, rest_api already creates a RESTClient instance for each of the resources under the hood.

One thing that I would like to discuss is that the PR extends to the resource configuration with the client key which, in turn, creates an additional way to configure the paginator. This is already configured via endpoint:

{
    "client": {
        "base_url": "https://api.mything.com",
        "auth": {
            # ...
        },
        "paginator": "json_link"
    },
    "resources": [
        "resource-using-bearer-auth",
        {
            "name": "my-resource-with-special-auth",
            "client": {
                "auth": HttpBasicAuth("user", dlt.secrets["your_basic_auth_password"]),
                "paginator": "json_link"  # <-- now it could be configured here as well
            },
            "endpoint": {
                "paginator": "json_link"
            }
        }
    ]
}

I think it would be better to keep the configuration in one place. To achieve this, I see two options:

Move the paginator configuration to the resources's client from the endpoint configuration.
Do not add a client configuration to resources, but extend the endpoint configuration to accept auth.

Although I think the first option is more consistent with the global client configuration, I would prefer the second option because:

It is shorter for cases where the user needs to configure pagination (which happens often).
It is backward-compatible.

Your example would then look like this:

{
    "client": {
        "base_url": "https://api.mything.com",
        "auth": {
            "type": "bearer",
            "token": dlt.secrets["your_api_token"],
        }
    },
    "resources": [
        "resource-using-bearer-auth",
        {
            "name": "my-resource-with-special-auth",
            "endpoint": {
                "path": "my-resource-with-special-auth",
                "auth": HttpBasicAuth("user", dlt.secrets["your_basic_auth_password"]),
                "paginator": "json_link",
            }
        }
    ]
}

What do you think?

joscha · 2024-11-24T22:42:37Z

Hi @burnash,

I initially had only auth as it was enough for what I wanted to do initially but when I opened the PR I noticed that this would not actually allow a completely reconfigured rest client. Some APIs when migrating between versions may choose to use different header parameters and/or base_urls so I thought overwriting/merging the whole client object would be best, as this would first of all work for all eventualities but also in concept be more simple than just add one separate object and then later another one, etc. whilst having to stay backwards compatible.
What do you think about removing the paginator as a possible property for overrides of the client parameters? Would that be an option? I do generally think your outlined 1. is cleaner long-term. If we were to accept both we could get the benefit of the short notation. Given the endpoint is more deeply nested I think the behavior would be that the endpoint paginator would override resource paginator would override global client paginator.
It is quite some baggage to carry though, for the convenience of omitting one extra client {}.

francescomucio · 2024-11-25T10:11:01Z

For my experience, changing the paginator is something more common than changing the authentication method for different endpoints of the same API.

I am in favour of the first option, but I can see how it can cause some confusion. In my opinion we should make clear how things are handled, possible options are:

the client in the resource cannot contain a client.paginator, but only an endpoint.paginator
the client in the resource can contain a client.paginator, but the endpoint.paginator wins.

So it will be like

client.paginator < default_resources.client.paginator < default_resource.endpoint.paginator < resource.client.paginator < resource.endpoint.paginator

What do you think?

joscha · 2024-11-25T10:44:33Z

For my experience, changing the paginator is something more common than changing the authentication method for different endpoints of the same API.

I am in favour of the first option, but I can see how it can cause some confusion. In my opinion we should make clear how things are handled, possible options are:

the client in the resource cannot contain a client.paginator, but only an endpoint.paginator

the client in the resource can contain a client.paginator, but the endpoint.paginator wins.

So it will be like

client.paginator < default_resources.client.paginator < default_resource.endpoint.paginator < resource.client.paginator < resource.endpoint.paginator

What do you think?

I think either of these are okay. Personally I'd most likely change the type of the accepted client config in the resource to NOT accept a paginator (option 1).

burnash · 2024-11-25T14:15:31Z

Thank you @joscha and @francescomucio for your thoughtful input and discussion on this PR. After considering all the points raised, I wanted to share my perspective.

I'm not fully convinced that adding a client configuration directly to the resource is the optimal approach. In practice, setting the paginator explicitly per endpoint is much more common than changing the authentication method within the same API. Introducing a client config at the resource level may lead to unnecessary ambiguity.

Regarding the headers, I believe these should be specified within the endpoint configuration. In fact, there's already a PR adding headers at the endpoint level #2084

As for the base_url, I'm not sure it's necessary within the resource configuration since endpoint.path can accept an absolute URL (I'd need update the documentation to reflect this). Putting headers and auth into the endpoint configuration would make it consistent with how the underlying RESTClient and RESTClient.paginate operate.

Here's my proposal for moving forward.

Extend the endpoint configuration to accept auth which overrides the client's auth configuration for the endpoint.
Add headers configuration to the endpoint via FEAT: parameterised headers rest_api_source #2084

Here's how the configuration would look:

{
    "client": {
        "base_url": "https://api.mything.com",
        "auth": {
            "type": "bearer",
            "token": dlt.secrets["your_api_token"],
        }
    },
    "resources": [
        "resource-using-bearer-auth",
        {
            "name": "my-resource-with-special-auth",
            "endpoint": {
                "path": "my-resource-with-special-auth",
                "auth": HttpBasicAuth("user", dlt.secrets["your_basic_auth_password"]),
                "paginator": "json_link",
                "headers": {"Custom-Header": "value"}
            }
        }
    ]
}

@joscha, would you be open to updating the PR to reflect this approach?

joscha · 2024-11-25T14:23:33Z

I can most definitely update the PR that way if that's what you wish. I think possibly spreading the individual properties of Client into the endpoint config is shortsighted as it will lead to an explosion of highly specialized properties spread into that object of whenever another is missing to support the next person's use case.
But I haven't DLT long enough to say whether that is likely or not, so given that, unless I hear back from you otherwise, I will update the PR later to use auth and headers.
I will also update the docs with the mention of the absolute URL as it is a related issue, I was not aware this was accepted before you wrote it.
Thank you!

joscha · 2024-11-25T14:35:58Z

Purely in regards to mechanics, do you: * Want me to wait until the open headers PR is decided and merged * Open the PR anyway and we deal with conflicts * Want me to base my PR off the headers PR ?

…

On Mon, Nov 25, 2024, 14:15 Anton Burnashev ***@***.***> wrote: Thank you @joscha <https://github.com/joscha> and @francescomucio <https://github.com/francescomucio> for your thoughtful input and discussion on this PR. After considering all the points raised, I wanted to share my perspective. I'm not fully convinced that adding a client configuration directly to the resource is the optimal approach. In practice, setting the paginator explicitly per endpoint is much more common than changing the authentication method within the same API. Introducing a client config at the resource level may lead to unnecessary ambiguity. Regarding the headers, I believe these should be specified within the endpoint configuration. In fact, there's already a PR adding headers at the endpoint level #2084 <#2084> As for the base_url, I'm not sure it's necessary within the resource configuration since endpoint.path can accept an absolute URL (I'd need update the documentation to reflect this). Putting headers and auth into the endpoint configuration would make it consistent with how the underlying RESTClient and RESTClient.paginate operate. Here's my proposal for moving forward. 1. Extend the endpoint configuration to accept auth which overrides the client's auth configuration for the endpoint. 2. Add headers configuration to the endpoint via #2084 <#2084> Here's how the configuration would look: { "client": { "base_url": "https://api.mything.com", "auth": { "type": "bearer", "token": dlt.secrets["your_api_token"], } }, "resources": [ "resource-using-bearer-auth", { "name": "my-resource-with-special-auth", "endpoint": { "path": "my-resource-with-special-auth", "auth": HttpBasicAuth("user", dlt.secrets["your_basic_auth_password"]), "paginator": "json_link", "headers": {"Custom-Header": "value"} } } ] } @joscha <https://github.com/joscha>, would you be open to updating the PR to reflect this approach? — Reply to this email directly, view it on GitHub <#2082 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABN5BQTIFFG2MKADD2KV7T2CMWJVAVCNFSM6AAAAABSF4PSF2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIOJYGEZTOMRQGI> . You are receiving this because you were mentioned.Message ID: ***@***.***>

burnash · 2024-11-25T14:37:53Z

Thank you @joscha for your understanding and flexibility! Let's proceed with adding just the auth configuration to the endpoint config for now. Since #2084 is already handling the headers implementation. I will help with the managing the conflicts.

joscha · 2024-11-26T00:25:47Z

changes in @burnash. Decided to do the mention of being able to use an absolute URL separately in pull request #2099.

joscha · 2024-11-26T12:34:18Z

I just saw that verified-sources seems to have a copy (?) of the rest client - see
https://github.com/dlt-hub/verified-sources/blob/3d2993a685e8c1d713a2283ac4025ca3903ed392/sources/rest_api/__init__.py#L249

is that intentional? (my assumption is yes, although none of the other verified sources seem to use it)
is there a process that syncs the two? If yes, where can I find it? if no, why not?
if 2.) is "no", should I make the same change in this repository as well?

burnash

Thanks you @joscha
Look good, there are a couple of small improvements needed, please the comments.

docs/website/docs/dlt-ecosystem/verified-sources/rest_api/basic.md

dlt/sources/rest_api/__init__.py

burnash · 2024-11-26T16:03:28Z

Very good question.

I just saw that verified-sources seems to have a copy (?) of the rest client - see https://github.com/dlt-hub/verified-sources/blob/3d2993a685e8c1d713a2283ac4025ca3903ed392/sources/rest_api/__init__.py#L249

is that intentional? (my assumption is yes, although none of the other verified sources seem to use it)

Yes, it's intentional, we moved rest_api, sql_database and filesystem sources to dlt core in 1.0.0. Versions prior to 1.0.0 still reference these sources as being in the verified-sources repository when you execute dlt init.

is there a process that syncs the two? If yes, where can I find it? if no, why not?

There isn’t a formal sync mechanism in place. If someone is working with projects that require or benefit from the latest updates or additional features available in the newer versions of these resources, I recommend upgrading to dlt version 1.0.0 or later.

if 2.) is "no", should I make the same change in this repository as well?

As this is a feature PR, for now, there's no need to port these changes to verified-sources repo. For bugfixes it's different.

…c.md Co-authored-by: Anton Burnashev <[email protected]>

Co-authored-by: Anton Burnashev <[email protected]>

tests/sources/rest_api/integration/test_response_actions.py

…des-per-endpoint

tests/sources/rest_api/configurations/source_configs.py

joscha · 2024-11-27T10:12:58Z

🎉 all good to go now

burnash

🎉 all good to go now

Yes, looks great, thank you!

joscha · 2024-11-27T12:29:21Z

Awesome, thank you. Are there instructions on how to use the current devel branch at a given sha? The only mention of nightly builds I could find is here: #997

burnash · 2024-11-27T12:59:37Z

You can use git+ prefix in pip, for example:

Install a devel branch:

pip install "git+https://github.com/dlt-hub/dlt.git@devel"

Install a devel branch with duckdb extra

pip install "git+https://github.com/dlt-hub/dlt.git@devel#egg=dlt[duckdb]"

Replace devel with an sha to get a specific commit.

joscha commented Nov 21, 2024

View reviewed changes

tests/sources/rest_api/configurations/source_configs.py Show resolved Hide resolved

francescomucio reviewed Nov 21, 2024

View reviewed changes

dlt/sources/rest_api/__init__.py Outdated Show resolved Hide resolved

francescomucio reviewed Nov 21, 2024

View reviewed changes

joscha commented Nov 21, 2024

View reviewed changes

dlt/sources/rest_api/__init__.py Outdated Show resolved Hide resolved

joscha force-pushed the joscha/auth-overrides-per-endpoint branch from 4cbf8de to 1bf1fe7 Compare November 21, 2024 20:53

burnash self-assigned this Nov 24, 2024

feat(rest_api): custom client for specific resources

dff62c9

joscha force-pushed the joscha/auth-overrides-per-endpoint branch from 1bf1fe7 to dff62c9 Compare November 26, 2024 00:24

joscha mentioned this pull request Nov 26, 2024

docs: document that path can also be a URL #2099

Merged

burnash requested changes Nov 26, 2024

View reviewed changes

joscha and others added 3 commits November 27, 2024 03:04

Update docs/website/docs/dlt-ecosystem/verified-sources/rest_api/basi…

56d4e44

…c.md Co-authored-by: Anton Burnashev <[email protected]>

Update dlt/sources/rest_api/__init__.py

d065d0f

Co-authored-by: Anton Burnashev <[email protected]>

Update dlt/sources/rest_api/__init__.py

c3f1834

Co-authored-by: Anton Burnashev <[email protected]>

joscha commented Nov 26, 2024

View reviewed changes

tests/sources/rest_api/integration/test_response_actions.py Outdated Show resolved Hide resolved

style: fix lint

aa0a9d8

Merge remote-tracking branch 'upstream/devel' into joscha/auth-overri…

0365242

…des-per-endpoint

joscha requested a review from burnash November 26, 2024 22:39

style: remove superfluous assignment

e2f6c8c

burnash reviewed Nov 27, 2024

View reviewed changes

tests/sources/rest_api/configurations/source_configs.py Outdated Show resolved Hide resolved

Fix failing test

b80734e

joscha requested a review from burnash November 27, 2024 10:13

burnash approved these changes Nov 27, 2024

View reviewed changes

burnash merged commit aa80667 into dlt-hub:devel Nov 27, 2024
59 checks passed

joscha deleted the joscha/auth-overrides-per-endpoint branch November 27, 2024 12:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(rest_api): custom client for specific resources #2082

feat(rest_api): custom client for specific resources #2082

joscha commented Nov 21, 2024 •

edited

Loading

joscha Nov 21, 2024

netlify bot commented Nov 21, 2024 •

edited

Loading

joscha Nov 21, 2024

francescomucio Nov 21, 2024

joscha Nov 21, 2024

francescomucio Nov 21, 2024

joscha Nov 21, 2024

joscha commented Nov 22, 2024

burnash commented Nov 24, 2024

joscha commented Nov 24, 2024

francescomucio commented Nov 25, 2024

joscha commented Nov 25, 2024

burnash commented Nov 25, 2024

joscha commented Nov 25, 2024

joscha commented Nov 25, 2024 via email

burnash commented Nov 25, 2024

joscha commented Nov 26, 2024 •

edited

Loading

joscha commented Nov 26, 2024

burnash left a comment

burnash commented Nov 26, 2024

joscha commented Nov 27, 2024

burnash left a comment

joscha commented Nov 27, 2024

burnash commented Nov 27, 2024

feat(rest_api): custom client for specific resources #2082

feat(rest_api): custom client for specific resources #2082

Conversation

joscha commented Nov 21, 2024 • edited Loading

Description

Example

Related Issues

Additional Context

joscha Nov 21, 2024

Choose a reason for hiding this comment

netlify bot commented Nov 21, 2024 • edited Loading

✅ Deploy Preview for dlt-hub-docs canceled.

joscha Nov 21, 2024

Choose a reason for hiding this comment

francescomucio Nov 21, 2024

Choose a reason for hiding this comment

joscha Nov 21, 2024

Choose a reason for hiding this comment

francescomucio Nov 21, 2024

Choose a reason for hiding this comment

joscha Nov 21, 2024

Choose a reason for hiding this comment

joscha commented Nov 22, 2024

burnash commented Nov 24, 2024

joscha commented Nov 24, 2024

francescomucio commented Nov 25, 2024

joscha commented Nov 25, 2024

burnash commented Nov 25, 2024

joscha commented Nov 25, 2024

joscha commented Nov 25, 2024 via email

burnash commented Nov 25, 2024

joscha commented Nov 26, 2024 • edited Loading

joscha commented Nov 26, 2024

burnash left a comment

Choose a reason for hiding this comment

burnash commented Nov 26, 2024

joscha commented Nov 27, 2024

burnash left a comment

Choose a reason for hiding this comment

joscha commented Nov 27, 2024

burnash commented Nov 27, 2024

joscha commented Nov 21, 2024 •

edited

Loading

netlify bot commented Nov 21, 2024 •

edited

Loading

joscha commented Nov 26, 2024 •

edited

Loading