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

Add documentation examples for loading credentials externally #2299

Merged
merged 10 commits into from
Feb 8, 2023

Conversation

AhdraMeraliQB
Copy link
Contributor

@AhdraMeraliQB AhdraMeraliQB commented Feb 8, 2023

Signed-off-by: Ahdra Merali ahdra.merali@quantumblack.com

NOTE: Kedro datasets are moving from kedro.extras.datasets to a separate kedro-datasets package in
kedro-plugins repository. Any changes to the dataset implementations
should be done by opening a pull request in that repository.

Description

This PR resolves issue #2148.

We often get requests on how to use credentials managers with Kedro, especially in platforms like Azure, AWS or GCP. This has always been possible, but not easy to come up with on your own and we haven't had any documentation on it. With the new changes on the AbstractConfigLoader and the addition of the after_context_created hook, this has become extremely easy, but will remain unknown to our users unless we document it.

Checklist

  • Read the contributing guidelines
  • Opened this PR as a 'Draft Pull Request' if it is work-in-progress
  • Updated the documentation to reflect the code changes
  • Added a description of this change in the RELEASE.md file
  • Added tests to cover my changes

Ahdra Merali added 2 commits February 8, 2023 09:59
Signed-off-by: Ahdra Merali <ahdra.merali@quantumblack.com>
Signed-off-by: Ahdra Merali <ahdra.merali@quantumblack.com>
Ahdra Merali added 2 commits February 8, 2023 10:11
Signed-off-by: Ahdra Merali <ahdra.merali@quantumblack.com>
Signed-off-by: Ahdra Merali <ahdra.merali@quantumblack.com>
@AhdraMeraliQB AhdraMeraliQB marked this pull request as ready for review February 8, 2023 10:22
@AhdraMeraliQB AhdraMeraliQB removed the request for review from yetudada February 8, 2023 10:22
@@ -113,3 +113,70 @@ def after_dataset_loaded(self, dataset_name: str, data: Any) -> None:
end = time.time()
self._logger.info("Loading dataset %s ended at %0.3f", dataset_name, end)
```

## Use Hooks to load external credentials
We recommend using the `after_context_created` Hook to add credentials to the session's config loader instance. In this example we show how to load credentials from Azure KeyVault.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Link to KeyVault?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it worth saying something like other stores are supported (examples) and explain what differences there may be (if any)?

Copy link
Contributor

@stichbury stichbury left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! I added a couple of comments of revisions and maybe we could put something about other key vaults being supported (if they are ?) but that's a lower priority.

AhdraMeraliQB and others added 4 commits February 8, 2023 11:01
Co-authored-by: Jo Stichbury <jo_stichbury@mckinsey.com>
Signed-off-by: Ahdra Merali <ahdra.merali@quantumblack.com>
Signed-off-by: Ahdra Merali <ahdra.merali@quantumblack.com>
Copy link
Contributor

@SajidAlamQB SajidAlamQB left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great! Just some minor suggestions. 🌟

}
```

And finally, don't forget to add the hook to your `settings.py` file:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add this link here to our docs on how to register hook implementations.

Ahdra Merali and others added 2 commits February 8, 2023 11:20
@AhdraMeraliQB AhdraMeraliQB merged commit 1124fb7 into main Feb 8, 2023
@AhdraMeraliQB AhdraMeraliQB deleted the docs/add-inject-creds-with-hooks-example branch February 8, 2023 14:15
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.

Add documentation examples for loading credentials externally
3 participants