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

Minor: improve CatalogProvider documentation with rationale and info about remote catalogs #8968

Merged
merged 1 commit into from
Jan 24, 2024

Conversation

alamb
Copy link
Contributor

@alamb alamb commented Jan 23, 2024

Which issue does this PR close?

related to #7013

Rationale for this change

The topic about how to implement remote catalogs has come up several times (most recently in #8887) so it would be good to update the docs to provide more detail

What changes are included in this PR?

  1. Improve docstrings for various catalog related traits
  2. Add background, examples, and notes on how to implement a remote catalog to [CatalogProvider]

Are these changes tested?

covered by CI tests

Are there any user-facing changes?

Better docs

@github-actions github-actions bot added the core Core DataFusion crate label Jan 23, 2024
/// Represent a list of named [`CatalogProvider`]s.
///
/// Please see the documentation on `CatalogProvider` for details of
/// implementing a custom catalog.
pub trait CatalogList: Sync + Send {
Copy link
Contributor

Choose a reason for hiding this comment

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

perhaps CatalogProviderList will be a better name? I can do this in following PR

Copy link
Contributor

Choose a reason for hiding this comment

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

Filed #9002

Copy link
Contributor

@comphead comphead left a comment

Choose a reason for hiding this comment

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

lgtm thanks @alamb

@comphead comphead merged commit 90e61c7 into apache:main Jan 24, 2024
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Core DataFusion crate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants