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

Docs: add data source configuration guide #1118

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

Conversation

josmperez
Copy link
Contributor

Add guide for data source plugin configuration

@josmperez josmperez added type/docs Changes only affect the documentation no-changelog Don't include in changelog and version calculations labels Sep 13, 2024
@josmperez josmperez self-assigned this Sep 13, 2024
Copy link

github-actions bot commented Sep 13, 2024

Hello! 👋 This repository uses Auto for releasing packages using PR labels.

✨ This PR can be merged. It will not be considered when calculating future versions of the npm packages and will not appear in the changelogs.

@josmperez josmperez marked this pull request as ready for review October 11, 2024 01:30
@josmperez josmperez requested a review from a team as a code owner October 11, 2024 01:30
@josmperez josmperez requested review from mckn and removed request for a team October 11, 2024 01:30
Copy link
Contributor Author

@josmperez josmperez left a comment

Choose a reason for hiding this comment

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

Needs response from PM plus some clean up from the writer (me).

Copy link
Contributor Author

@josmperez josmperez left a comment

Choose a reason for hiding this comment

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

Needs response from PM plus some clean up from the writer (me).

Copy link
Collaborator

@mckn mckn left a comment

Choose a reason for hiding this comment

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

Overall I think this looks good! We should probably get some more eyes on the Auth component parts.


## Authentication component

Use the `Authentication` component for handling authentication on the configuration page. This component is already wrapped inside a `ConfigSection`, so additional wrapping is unnecessary.
Copy link
Member

Choose a reason for hiding this comment

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

Let's remove all calls to use Authentication and keep only DataSourceHttpSettings without indicating that is deprecated (it is not deprecated)


Additional settings are optional and provide users with extra control over the data source plugin. These settings should:

- Be placed in a separate section using the `ConfigSection` component.
Copy link
Member

@academo academo Nov 8, 2024

Choose a reason for hiding this comment

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

As we are going to keep (for now) the documentation asking to use DataSourceHttpSettings let's remove references to the ConfigSection and ConfigSubSection component

Copy link
Member

@academo academo left a comment

Choose a reason for hiding this comment

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

DataSourceHttpSettings is not deprecated and it is a simple component that works for most cases. If we want to encourage people to use the other components available for authentication and configuration we should maybe create a separate "advance" configuration.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-changelog Don't include in changelog and version calculations type/docs Changes only affect the documentation
Projects
Status: 🧑‍💻 In development
Development

Successfully merging this pull request may close these issues.

5 participants