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

Cleanup CLI flags and options for InfluxDB 3 #25646

Open
mgattozzi opened this issue Dec 12, 2024 · 5 comments · May be fixed by #25737
Open

Cleanup CLI flags and options for InfluxDB 3 #25646

mgattozzi opened this issue Dec 12, 2024 · 5 comments · May be fixed by #25737
Assignees
Labels

Comments

@mgattozzi
Copy link
Contributor

Use case:
A better user experience tailored towards InfluxDB 3 OSS version

Proposal:
Roll our own clap_blocks crate for OSS

Current behaviour:
Currently we have clap options set by reusing clap_blocks from IOx and it reflects so in the comments which can be confusing and it includes many options we do not need for OSS that can make it hard for users to discover what options are available to them.

Desired behaviour:

  • CLI options with help text specific to the OSS version
  • CLI options that contain only the necessary flags for OSS
  • Removal of clap_block dependency from core

Alternatives considered:
We can also just do them inline in influxdb3 or not at all. With internal feedback however this is considered confusing and verbose and not doing it all is not as viable an option as other users are likely to experience the same confusion.

@mgattozzi mgattozzi added the v3 label Dec 12, 2024
@hiltontj
Copy link
Contributor

We could also use Arg::hide to not display blocks from IOx, i.e.,

#[clap(hide = true)]

but then, I am not sure if there is a way for users to see those blocks at all, unless they refer to some external provided documentation.

This would help maintain compatibility in the short term, e.g., with the perf team's test harness, which is using several of the clap blocks from IOx like object store, tracing, and logging (perhaps others).

@pauldix
Copy link
Member

pauldix commented Dec 12, 2024

The object store, tracing and log blocks should be relevant to Monolith, shouldn't they? Meaning we don't want to hide them.

@hiltontj
Copy link
Contributor

The object store, tracing and log blocks should be relevant to Monolith, shouldn't they? Meaning we don't want to hide them.

I think all the blocks we are using are relevant. But there is a lot of IOx verbiage in them and the associated environment variables all use the IOX_ prefix. So, hide'ing them is really just a quick and dirty solution. Ultimately, we need to go through the chore of porting over each block and only including what is relevant.

@pauldix
Copy link
Member

pauldix commented Dec 12, 2024

We should do the cleanup in advance of the alpha.

@mgattozzi
Copy link
Contributor Author

mgattozzi commented Dec 12, 2024

I think we can just copy the relevant parts that we need from clap_blocks and rewrite them or pare the options down a bit. If we hide them in core then we can't sync easily with IOx. In many ways it's a read only copy of parts of IOx

@mgattozzi mgattozzi self-assigned this Dec 13, 2024
mgattozzi added a commit that referenced this issue Jan 2, 2025
This removes references to IOx in our docs and comments as when we
started making Monolith last year it was a fork of influxdb_iox.
This was something that we changed in some places, but not all as
time went on. To avoid confusion whenever we more broadly release
this software I've removed references to IOx in our code and docs.
Note however that we still use a lot of IOx code and our system
tables also return iox as part of the name. This is a separate issue
namely #25227. Some things like env vars will also be cleaned up
in #25646

Closes #25662
mgattozzi added a commit that referenced this issue Jan 2, 2025
This removes references to IOx in our docs and comments as when we
started making Monolith last year it was a fork of influxdb_iox.
This was something that we changed in some places, but not all as
time went on. To avoid confusion whenever we more broadly release
this software I've removed references to IOx in our code and docs.
Note however that we still use a lot of IOx code and our system
tables also return iox as part of the name. This is a separate issue
namely #25227. Some things like env vars will also be cleaned up
in #25646

Closes #25662
mgattozzi added a commit that referenced this issue Jan 3, 2025
This makes quite a few major changes to our CLI and how users interact
with it:

1. All commands are now in the form <verb> <noun> this was to make the
   commands consistent. We had last-cache as a noun, but serve as a
   verb in the top level. Given that we could only create or delete
   All noun based commands have been move under a create and delete
   command
2. --host short form is now -H not -h which is reassigned to -h/--help
   for shorter help text and is in line with what users would expect
   for a CLI
3. Only the needed items from clap_blocks have been moved into
   `influxdb3_clap_blocks` and any IOx specific references were changed
   to InfluxDB 3 specific ones
4. References to InfluxDB 3.0 OSS have been changed to InfluxDB 3 Core
   in our CLI tools
5. --dbname has been changed to --database to be consistent with --table
   in many commands. The short -d flag still remains. In the create/
   delete command for the database however the name of the database is
   a positional arg

   e.g. `influxbd3 create database foo` rather than
        `influxdb3 database create --dbname foo`
6. --table has been removed from the delete/create command for tables
   and is now a positional arg much like database
7. clap_blocks was removed as dependency to avoid having IOx specific
   env vars

Unfortunately we have quite a few options to run the software and I
couldn't cut down on them, but at least with this commands and options
will be more discoverable and we have full control over our CLI options
now.

Closes #25646
@mgattozzi mgattozzi linked a pull request Jan 3, 2025 that will close this issue
mgattozzi added a commit that referenced this issue Jan 3, 2025
This makes quite a few major changes to our CLI and how users interact
with it:

1. All commands are now in the form <verb> <noun> this was to make the
   commands consistent. We had last-cache as a noun, but serve as a
   verb in the top level. Given that we could only create or delete
   All noun based commands have been move under a create and delete
   command
2. --host short form is now -H not -h which is reassigned to -h/--help
   for shorter help text and is in line with what users would expect
   for a CLI
3. Only the needed items from clap_blocks have been moved into
   `influxdb3_clap_blocks` and any IOx specific references were changed
   to InfluxDB 3 specific ones
4. References to InfluxDB 3.0 OSS have been changed to InfluxDB 3 Core
   in our CLI tools
5. --dbname has been changed to --database to be consistent with --table
   in many commands. The short -d flag still remains. In the create/
   delete command for the database however the name of the database is
   a positional arg

   e.g. `influxbd3 create database foo` rather than
        `influxdb3 database create --dbname foo`
6. --table has been removed from the delete/create command for tables
   and is now a positional arg much like database
7. clap_blocks was removed as dependency to avoid having IOx specific
   env vars

Unfortunately we have quite a few options to run the software and I
couldn't cut down on them, but at least with this commands and options
will be more discoverable and we have full control over our CLI options
now.

Closes #25646
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants