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

refactor: introduce influxdb3_types crate #25946

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

waynr
Copy link
Contributor

@waynr waynr commented Jan 31, 2025

Fixes #24672

...partially. It doesn't completely move all request/response types into the new crate. In particular, it leaves out all the *Builder types in the influxdb3_client crate, ie:

zsh/5 13797 [148]  (git)-[refactor/shared-types-crate|rebase-i]-% rg 'struct.*Req' influxdb3_server influxdb3_client
influxdb3_client/src/lib.rs
832:pub struct WriteRequestBuilder<'c, B> {
906:pub struct QueryRequestBuilder<'c> {
1112:pub struct ShowDatabasesRequestBuilder<'c> {
1157:pub struct CreateLastCacheRequestBuilder<'c> {
1250:pub struct CreateDistinctCacheRequestBuilder<'c> {

These builder types look like they are intended to produce the effect of a fluent interface for the influxdb3_client::Client type with typical usage that looks like:

let client = Client::new("http://localhost:8181")?;
client
    .api_v3_write_lp("db_name") // <- returns a WriteRequestBuilder
    .precision(Precision::Millisecond)
    .accept_partial(true)
    .body("cpu,host=s1 usage=0.5")
    .send() // <- constructs `WriteParams` to be used as a POST request body
    .await
    .expect("send write_lp request");

To achieve this effect, these builder types hold a reference to the Client itself, which means they can't easily be migrated outside of this module without significantly refactoring away from this fluent approach to making certain kinds of request.

What this PR does do:

  • move most HTTP req/resp types into influxdb3_types crate
  • removes the use of locally-scoped request type structs from the influxdb3_client crate
  • fix plugin dependency/package install bug
    • it looks like the DELETE http method was being used where POST was expected for /api/v3/configure/plugin_environment/install_packages and /api/v3/configure/plugin_environment/install_requirements

Fixes #24672

...partially. It doesn't completely move all request/response types into the new crate. In particular, it leaves out all the `*Builder` types in the `influxdb3_client` crate, ie:

```
zsh/5 13797 [148]  (git)-[refactor/shared-types-crate|rebase-i]-% rg 'struct.*Req' influxdb3_server influxdb3_client
influxdb3_client/src/lib.rs
832:pub struct WriteRequestBuilder<'c, B> {
906:pub struct QueryRequestBuilder<'c> {
1112:pub struct ShowDatabasesRequestBuilder<'c> {
1157:pub struct CreateLastCacheRequestBuilder<'c> {
1250:pub struct CreateDistinctCacheRequestBuilder<'c> {
```

These builder types look like they are intended to produce the effect of a fluent interface for the `influxdb3_client::Client` type with typical usage that looks like:

```rust
let client = Client::new("http://localhost:8181")?;
client
    .api_v3_write_lp("db_name") // <- returns a WriteRequestBuilder
    .precision(Precision::Millisecond)
    .accept_partial(true)
    .body("cpu,host=s1 usage=0.5")
    .send() // <- constructs `WriteParams` to be used as a POST request body
    .await
    .expect("send write_lp request");
```

To achieve this effect, these builder types hold a reference to the `Client` itself, which means they can't easily be migrated outside of this module without significantly refactoring away from this fluent approach to making certain kinds of request.

What this PR does do:

* move most HTTP req/resp types into `influxdb3_types` crate
* removes the use of locally-scoped request type structs from the `influxdb3_client` crate
* fix plugin dependency/package install bug
  * it looks like the `DELETE` http method was being used where `POST` was expected for `/api/v3/configure/plugin_environment/install_packages` and `/api/v3/configure/plugin_environment/install_requirements`
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.

Add crate to share types between influxdb3 server and client
1 participant