Feat/1596 adds custom docs config provider #1642
Conversation
…support to vault providers, refactors set_value in formet TomlBaseProvider
✅ Deploy Preview for dlt-hub-docs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
@VioletM here's preview of the example https://deploy-preview-1642--dlt-hub-docs.netlify.app/docs/examples/custom_config_provider |
| Implement and use your own config provider by just providing loader function. See [example](../../examples/custom_config_provider) for `yaml` based config that | ||
| supports switchable profiles. | ||
| ::: | ||
|
|
There was a problem hiding this comment.
I realize that the latter is already covered by the example, I'd still add two super simple examples of the BaseDocProvider and the CustomLoaderDocProvider here, but this could be done in a followup PR.
| provider = CustomLoaderDocProvider("profiles", functools.partial(loader, profile_name)) | ||
| # register provider, it will be added as the last one in chain | ||
| dlt.config.register_provider(provider) | ||
|
|
There was a problem hiding this comment.
I'd add a comment here that this is the place to run the pipeline (imho examples should always be thought of in the context of a pipeline execution)
| """ | ||
| self._name = name | ||
| self._supports_secrets = supports_secrets | ||
| super().__init__(loader()) |
There was a problem hiding this comment.
What is the use of this CustomLoaderProvider if it is not lazily evaluated on first access of the dict?
There was a problem hiding this comment.
heh you are right. in this implementation it does not make sense. BUT maybe we want to change it to lazy evaluation ie. on first use. may be necessary ie. on Airflow. so let's keep it like that!
|
|
||
| # instantiate custom provider using `prod` profile | ||
| # NOTE: all placeholders (ie. GITHUB_API_KEY) will be evaluated in next line! | ||
| provider = CustomLoaderDocProvider("profiles", functools.partial(loader, profile_name)) |
There was a problem hiding this comment.
I would not use stuff like partials in examples if it is not absolutely necessary. While I agree the code looks nicer this way, my intuition says this makes it harder to understand what is going on for less experienced devs. It's up to you what you think :)
sh-rp
left a comment
sh-rp
left a comment
There was a problem hiding this comment.
Super useful PR imho. I have a few comments, none of which are super important for this release, but the docs could be better / more helpful.
VioletM
left a comment
VioletM
left a comment
There was a problem hiding this comment.
This looks good!
I'd say we should add more docs and examples for CustomLoaderDocProvider. But it can be done in the separate PR later after we'll get the first feedback :)
| # instantiate custom provider using `prod` profile | ||
| # NOTE: all placeholders (ie. GITHUB_API_KEY) will be evaluated in next line! | ||
| provider = CustomLoaderDocProvider("profiles", functools.partial(loader, profile_name)) | ||
| # register provider, it will be added as the last one in chain |
There was a problem hiding this comment.
Last one in the chain means, that if they have secrets.toml or env variable with the same key it will override the explicitly registered CustomLoaderDocProvider provider, right?
There was a problem hiding this comment.
It just seems more intuitive for me that the custom provider I wrote to be the first in chain. But I guess it's OK, as long as it's stated in the docs
Description
fixes #1596 and indirectly #1629
allows to instantiate custom config provider with user supplied loading function which may process any file format from any location