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

feat: add the influxdb3_client crate #24665

Merged
merged 11 commits into from
Feb 16, 2024
Merged

feat: add the influxdb3_client crate #24665

merged 11 commits into from
Feb 16, 2024

Conversation

hiltontj
Copy link
Contributor

@hiltontj hiltontj commented Feb 12, 2024

Closes #24661, as this should allow us to build a barebones CLI, but some follow-on issues will likely be needed.

This adds a new crate: influxdb3_client, which provides a Client type for accessing the HTTP API of InfluxDB 3. Currently, with support for:

  • POST /api/v3/write_lp
  • GET /api/v3/query_sql

A test is included for each corresponding method to check/demonstrate usage, and each method also has a short doc comment with an example.

Implementation Notes

  • I took some major shortcuts on response handling. For write_lp, it looks to be just responding with "{}", so if/once there is some content being returned there we can handle it.
  • For query_sql, the client method just returns the raw Bytes, and the caller will have to handle. We may certainly get more sophisticated with this, but for the sake of the CLI, which will just stream the bytes to stdout anyways, I figured this acceptable.
  • We should centralize/re-use the URL parameter type definitions, e.g., QueryParams and WriteParams, currently I have re-defined them in this crate

A new crate, influxdb3_client, was added, which provides the Client
struct. This gives programmatic access to the infludxb3 HTTP API.

This commit implements a partial version of the api_v3_write_lp method
along with a test for ensuring the happy path. The underlying API is
not yet complete, so the response handling is still a TODO.
The new Client and types related to the Client::api_v3_write_lp method
have been documented

The stub for the write_lp_from_file test was removed, to be done later
@hiltontj hiltontj added the v3 label Feb 12, 2024
@hiltontj hiltontj self-assigned this Feb 12, 2024
The api_v3_query_sql method was added to the influxdb3 Client, which
allows caller to compose a request to the respective API.

Similar to the write_lp API, this uses a builder approach to composing
the request. The handling of the response was kept naive and just gives
back the bytes returned. We may improve this in future once the format
handling in the API is nailed down and we have JSON lines supported.
@hiltontj hiltontj marked this pull request as ready for review February 13, 2024 15:54
Switched reqwest to use rustls and disabled default features in mockito
Copy link
Contributor

@mgattozzi mgattozzi left a comment

Choose a reason for hiding this comment

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

@hiltontj I had two actual changes worth making: the stray crate name in Cargo.toml and the test formatting for the lp message. The rest are mostly comments. Otherwise this is solid, couldn't have written it better myself and the API should be real nice to use.


use bytes::Bytes;
use reqwest::{Body, IntoUrl, StatusCode};
use secrecy::{ExposeSecret, Secret};
Copy link
Contributor

Choose a reason for hiding this comment

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

TIL about the secrecy crate this is really cool

let status = resp.status();
let content = resp.bytes().await.map_err(Error::Bytes)?;
match status {
// TODO - handle the OK response content, return to caller, etc.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it should be an empty body. I'm not sure why we left it as just {}. @pauldix were we planning on returning more on a successful response in the future for the write_lp endpoint? I don't see a reason too, but I'm probably missing context.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I was wondering if there would be stats returned, e.g., number of lines inserted, or in the case of accept_partial = true, the rejected lines, etc.

Clean up some rustdoc comments and TODO comments

Rename the with_auth_header method to with_auth_token to distinguish
setting the token itself vs. the entire header; and rename the struct
field on Client accordingly

Indent the write_lp payload in the test code

Remove the reqwest::Client as a parameter to the Client new method
@hiltontj hiltontj requested a review from mgattozzi February 14, 2024 20:56
Copy link
Contributor

@mgattozzi mgattozzi left a comment

Choose a reason for hiding this comment

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

This looks good to me. I think the open questions about write_lp can be addressed either later or in my PR whenever I open that.

@hiltontj hiltontj merged commit 80505d2 into main Feb 16, 2024
11 checks passed
@hiltontj hiltontj deleted the hiltontj/influxdb3-client branch February 16, 2024 20:02
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 this pull request may close these issues.

Create influxdb3_client for programmatic access to HTTP API
2 participants