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

use ndc-sdk for printSchemaAndCapabilities #40

Merged
merged 3 commits into from
Oct 30, 2024

Conversation

BenoitRanque
Copy link
Collaborator

Use the ndc-sdk for printSchemaAndCapabilities

ndc-sdk exposes a function for this purpose.
Because that function is generic over something that implements ConnectorSetup, which has an associated type that must implement Connector, the CLI now relies on effectively the whole connector crate.

So, most code was moved to ndc-clickhouse-core.
ndc-clickhouse is mostly a shell, it imports the type that implements ConnectorSetup from ndc-clickhouse-core and then calls default_main with it.

Copy link

@hallettj hallettj left a comment

Choose a reason for hiding this comment

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

Looks good! This was much easier to review than I thought it would be seeing the changed files count.

Copy link
Collaborator Author

@BenoitRanque BenoitRanque left a comment

Choose a reason for hiding this comment

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

Not merging for now. Seems ndc-sdk-core brings in ndc-test, which brings in reqwest, which brings in native tls/openssl.
Tried not requesting default features, but no luck so far

@BenoitRanque
Copy link
Collaborator Author

Fixed it

@BenoitRanque BenoitRanque merged commit 4c9020d into main Oct 30, 2024
13 checks passed
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.

2 participants