feat: Cleanup CLI flags for InfluxDB 3 Core #25737

mgattozzi · 2025-01-03T21:47:34Z

This makes quite a few major changes to our CLI and how users interact
with it:

All commands are now in the form 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
--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
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
References to InfluxDB 3.0 OSS have been changed to InfluxDB 3 Core
in our CLI tools
--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
--table has been removed from the delete/create command for tables
and is now a positional arg much like database
clap_blocks was removed as dependency to avoid having IOx specific
env vars
--cache-name is now an optional positional arg for last_cache and meta_cache
last-cache/meta-cache commands are now last_cache and meta_cache respectively

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

influxdb3/src/commands/query.rs

pauldix

Overall I think it looks good. The new structure makes sense to me and I ran it a bit to see how the help output looked and felt.

Some updates to clean that up, but I'm 👍 on this. Will let @hiltontj sound off as well since he had set up the initial structure.

influxdb3/src/commands/create.rs

hiltontj

I left a few comments/suggestions. Looking really solid. I like the switch to using positional args for the resource name, and changing to influxdb3 verb noun feels right given that we only have create/delete. Though I commented on the last/meta cache commands which still use named options for the resource name.

I won't request changes since Paul already did, but I can take another look once you've had a chance to address stuff.

hiltontj · 2025-01-03T23:52:15Z

Cargo.toml

@@ -105,9 +108,11 @@ serde_json = "1.0.127"
 serde_urlencoded = "0.7.0"
 serde_with = "3.8.1"
 sha2 = "0.10.8"
+snafu = "0.8"


Would be nice if we could factor out snafu since we use thiserror everywhere else, but that can be done later.

Yeah this was just to get everything working with what I copied from clap blocks. I've opened up an issue here

influxdb3/src/commands/common.rs

influxdb3/src/commands/delete.rs

influxdb3/src/commands/common.rs

influxdb3/src/commands/create.rs

influxdb3/src/commands/delete.rs

influxdb3/src/commands/create.rs

This implements the WAL plugin test API. It also introduces a new API for the Python plugins to be called, get their data, and call back into the database server. There are some things that I'll want to address in follow on work: * CLI tests, but will wait on #25737 to land for a refactor of the CLI here * Would be better to hook the Python logging to call back into the plugin return state like here: https://pyo3.rs/v0.23.3/ecosystem/logging.html#the-python-to-rust-direction * We should only load the LineBuilder interface once in a module, rather than on every execution of a WAL plugin * More tests all around But I want to get this in so that the actual plugin and trigger system can get udated to build around this model.

mgattozzi · 2025-01-06T18:58:09Z

@pauldix, @hiltontj, and @peterbarnett03 I've made all of the requested changes and have incorporated the plugin and trigger commands from main into this PR as well. It will need another thorough review

hiltontj

Two things otherwise it is looking good!

influxdb3/src/commands/deactivate.rs

This implements the WAL plugin test API. It also introduces a new API for the Python plugins to be called, get their data, and call back into the database server. There are some things that I'll want to address in follow on work: * CLI tests, but will wait on #25737 to land for a refactor of the CLI here * Would be better to hook the Python logging to call back into the plugin return state like here: https://pyo3.rs/v0.23.3/ecosystem/logging.html#the-python-to-rust-direction * We should only load the LineBuilder interface once in a module, rather than on every execution of a WAL plugin * More tests all around But I want to get this in so that the actual plugin and trigger system can get udated to build around this model.

* feat: Implement WAL plugin test API This implements the WAL plugin test API. It also introduces a new API for the Python plugins to be called, get their data, and call back into the database server. There are some things that I'll want to address in follow on work: * CLI tests, but will wait on #25737 to land for a refactor of the CLI here * Would be better to hook the Python logging to call back into the plugin return state like here: https://pyo3.rs/v0.23.3/ecosystem/logging.html#the-python-to-rust-direction * We should only load the LineBuilder interface once in a module, rather than on every execution of a WAL plugin * More tests all around But I want to get this in so that the actual plugin and trigger system can get udated to build around this model. * refactor: PR feedback

hiltontj

LGTM 🚀

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 8. --cache-name is now an optional positional arg for last_cache and meta_cache 9. last-cache/meta-cache commands are now last_cache and meta_cache respectively 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

_Follows #25737 (keeping in draft until that merges)_ Closes #25745 This PR provides both a CLI and underlying API for listing databases in the InfluxDB 3 Core server. Details are below. There was already a method to list databases for the query executor for InfluxQL; this works by exposing that via the `HttpApi` in `influxdb3_server`. However, one thing that we may address is that the query result for that uses `iox::database` as the column name. If we are removing references to `iox`, then we may want to just have it as `database`. I left it as is, for now, because I wanted to keep code churn down and wasn't sure why we use that prefix in the first place for the `SHOW DATABASES` and `SHOW RETENTION POLICIES` InfluxQL queries. ## Details ### CLI This PR provides the `influxdb3 show` CLI: ``` influxdb3 show -h List resources on the InfluxDB 3 Core server Usage: influxdb3 show <COMMAND> Commands: databases List databases help Print this message or the help of the given subcommand(s) Options: -h, --help Print help information ``` with the ability to list databases: ``` influxdb3 show databases -h List databases Usage: influxdb3 show databases [OPTIONS] Options: -H, --host <HOST_URL> The host URL of the running InfluxDB 3 Core server [env: INFLUXDB3_HOST_URL=] [default: http://127.0.0.1:8181] --token <AUTH_TOKEN> The token for authentication with the InfluxDB 3 Core server [env: INFLUXDB3_AUTH_TOKEN=] --show-deleted Include databases that were marked as deleted in the output --format <OUTPUT_FORMAT> The format in which to output the list of databases [default: pretty] [possible values: pretty, json, json_lines, csv] -h, --help Print help information ``` Since this uses the query executor, we can pass a `--format` argument to get the output as JSON, CSV, or JSONL, but by default, it uses the `pretty` format: ``` influxdb3 show databases +---------------+ | iox::database | +---------------+ | bar | +---------------+ ``` The `--show-deleted` flag will have the `deleted` column displayed as well as any databases that have been marked as deleted: ``` influxdb3 show databases --show-deleted +---------------------+---------+ | iox::database | deleted | +---------------------+---------+ | bar | false | | foo-20250105T202949 | true | +---------------------+---------+ ``` ### API The API to list databases can be invoked via: ``` GET /api/v3/configure/database ``` with optional parameters: * `format`: `pretty`, `json`, `csv`, `parquet`, or `jsonl` * `show_deleted`: `bool`, defaults to `false` Note that `database` is singular in the API endpoint, to be consistent with the other database related create/delete API endpoints. We could change it to be plural `databases` if that is the convention we want to go with.

In #25737 token creation was broken as a client would attempt to be made, but hit an unreachable branch. The fix was to just make the branch create a Client that won't do anything. We also add a test to make sure that if there is a regression we'll catch it in the future. Closes #25764

* fix: get create token subcommand to work again In #25737 token creation was broken as a client would attempt to be made, but hit an unreachable branch. The fix was to just make the branch create a Client that won't do anything. We also add a test to make sure that if there is a regression we'll catch it in the future. Closes #25764 * fix: make client turn a duration into seconds/u64 This commit fixes the creation of a metadata cache made via the cli by actually sending a time in seconds as a u64 and not as a duration. With that we can now make metadata caches again when the `--max-age` flag is specified. This commit also adds a regression test for the CLI so that we can catch this again in the future.

mgattozzi requested review from pauldix, hiltontj, jacksonrnewhouse and praveen-influx January 3, 2025 21:47

mgattozzi force-pushed the mgattozzi/flag-cleanup branch from 7ed5bcf to e6eb6e2 Compare January 3, 2025 22:15

peterbarnett03 reviewed Jan 3, 2025

View reviewed changes

influxdb3/src/commands/query.rs Outdated Show resolved Hide resolved

pauldix requested changes Jan 3, 2025

View reviewed changes

hiltontj reviewed Jan 4, 2025

View reviewed changes

pauldix reviewed Jan 4, 2025

View reviewed changes

influxdb3/src/commands/create.rs Outdated Show resolved Hide resolved

pauldix reviewed Jan 4, 2025

View reviewed changes

influxdb3/src/commands/create.rs Outdated Show resolved Hide resolved

hiltontj mentioned this pull request Jan 5, 2025

feat: show databases CLI/API #25748

Merged

pauldix mentioned this pull request Jan 5, 2025

feat: Implement WAL plugin test API #25704

Merged

mgattozzi mentioned this pull request Jan 6, 2025

Remove snafu from dependencies in favor of thiserror #25753

Open

mgattozzi force-pushed the mgattozzi/flag-cleanup branch from e6eb6e2 to 307094f Compare January 6, 2025 18:56

mgattozzi requested review from pauldix, hiltontj and peterbarnett03 January 6, 2025 18:57

mgattozzi force-pushed the mgattozzi/flag-cleanup branch from 307094f to d2ed342 Compare January 6, 2025 19:40

hiltontj reviewed Jan 6, 2025

View reviewed changes

influxdb3/src/commands/deactivate.rs Outdated Show resolved Hide resolved

influxdb3/src/commands/deactivate.rs Outdated Show resolved Hide resolved

mgattozzi requested a review from hiltontj January 6, 2025 21:01

mgattozzi force-pushed the mgattozzi/flag-cleanup branch from 7e15af1 to dd95922 Compare January 6, 2025 21:27

hiltontj approved these changes Jan 6, 2025

View reviewed changes

mgattozzi force-pushed the mgattozzi/flag-cleanup branch from dd95922 to a27ae65 Compare January 6, 2025 23:37

pauldix approved these changes Jan 6, 2025

View reviewed changes

mgattozzi merged commit f793d31 into main Jan 6, 2025
14 checks passed

mgattozzi deleted the mgattozzi/flag-cleanup branch January 6, 2025 23:51

mgattozzi mentioned this pull request Jan 9, 2025

fix: get create token subcommand to work again #25773

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Cleanup CLI flags for InfluxDB 3 Core #25737

feat: Cleanup CLI flags for InfluxDB 3 Core #25737

mgattozzi commented Jan 3, 2025 •

edited

Loading

pauldix left a comment

hiltontj left a comment

hiltontj Jan 3, 2025

mgattozzi Jan 6, 2025

mgattozzi commented Jan 6, 2025

hiltontj left a comment

hiltontj left a comment

feat: Cleanup CLI flags for InfluxDB 3 Core #25737

feat: Cleanup CLI flags for InfluxDB 3 Core #25737

Conversation

mgattozzi commented Jan 3, 2025 • edited Loading

pauldix left a comment

Choose a reason for hiding this comment

hiltontj left a comment

Choose a reason for hiding this comment

hiltontj Jan 3, 2025

Choose a reason for hiding this comment

mgattozzi Jan 6, 2025

Choose a reason for hiding this comment

mgattozzi commented Jan 6, 2025

hiltontj left a comment

Choose a reason for hiding this comment

hiltontj left a comment

Choose a reason for hiding this comment

mgattozzi commented Jan 3, 2025 •

edited

Loading