-
Notifications
You must be signed in to change notification settings - Fork 7
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
rfc: rough guidelines for cli plugins #92
Open
0x777
wants to merge
5
commits into
main
Choose a base branch
from
cli-guidelines
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
ccc734f
rfc: rough guidelines for cli plugins
0x777 2a33ac8
Merge branch 'main' of https://github.com/hasura/ndc-hub into cli-gui…
SandeepSamba 4d48c56
Add Plugin Invocation Spec
SandeepSamba 7e98529
Fix indent
SandeepSamba 11be49a
Add CLI publishing spec
SandeepSamba File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,53 @@ | ||
### CLI Plugin Guidelines | ||
|
||
Connectors can optionally provide a CLI plugin to help with author the | ||
connector's configuration. These are the broad categories of commands that | ||
could be useful: | ||
|
||
1. init (optional) | ||
|
||
Initializing the config directory for a connector. There could be | ||
more than one template. | ||
|
||
Let's ignore this for now (out of beta's scope) as the cli's `add | ||
ConnectorManifest` would also initialize the connector's config from the | ||
connector's default template. | ||
|
||
2. update | ||
|
||
A category of commands that modifies connector's config. For example, when | ||
working on Postgres, you may want to introspect the database and import all | ||
the tables into the connector's config. | ||
|
||
These are just some examples (they are in no way prescriptive of what the | ||
exact interface should be): | ||
|
||
```bash | ||
# 1. Imports the database and updates all the tables that are # not in pg_* | ||
schemas or information_schema | ||
# 2. Will also preserve the user written native queries | ||
<pg-plugin> update-config --replace | ||
|
||
# Same as above but is restricting the introspection to 'public' schema | ||
<pg-plugin> update-config --include-schema public | ||
``` | ||
|
||
The idea is that the user would specify a specific incantation of the update | ||
command that cli would then call at a fixed interval when the user invokes | ||
`h3 dev`. | ||
|
||
> [!NOTE] | ||
> In the case of Postgres connector, the whole `configureOptions` section | ||
> goes away from the connector's config because that doesn't affect the | ||
> NDC interface's behaviour and ideally part of update-config's arguments | ||
|
||
3. validate | ||
|
||
This should help validate the config for the connector. Note that the | ||
connector's deployment fails if the config is invalid. However for a user it | ||
may be easier to have a command that validates the config before it is | ||
deployed. It isn't strictly necessary for beta but a nice to have. | ||
|
||
In future, you could also have specific commands to validate parts of the | ||
config such as native queries. | ||
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems like it has the potential to duplicate functionality in the future... which path should the user follow, CLI's
add ConnectorManifest
or thisinit
? When should the user choose one over the other?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe
add ConnectorManifest
should be built on top of this (ie uses this to provide the init-ing)?