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

rfc: rough guidelines for cli plugins #92

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

rfc: rough guidelines for cli plugins #92

wants to merge 5 commits into from

Conversation

0x777
Copy link
Member

@0x777 0x777 commented Feb 2, 2024

Comment on lines +12 to +14
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.
Copy link
Collaborator

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 this init? When should the user choose one over the other?

Copy link
Collaborator

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)?

This could also be a totally different implementation. This is a convenience
command for the CLI to use during `h3 dev` so that the CLI can invoke this once,
and terminate it once the `dev` command has ended.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd like to better understand this.
The most naive, straightforward implementation is to literally run update in a loop.
Given update introspects the database, which may be intensive, this could get quite spammy.

Another option is to loop every X seconds.

A third is to only run the update when a file in the config directory is changed.

None of these seem satisfactory to me. In practice, the user is unlikely to be rapidly modifying their database,
so it's likely not necessary to run the update that often. Changes to the config directory probably don't mean the db has changed, so that option also does not make sense to me.

So, how should this be implemented? Again, happy to literally implement loop { update() } as stated here but I really doubt we want that.

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.

4 participants