influxdata / influxdb

Scalable datastore for metrics, events, and real-time analytics
https://influxdata.com
Apache License 2.0
28.82k stars 3.55k forks source link

Influxdb3 API V2 POST endpoint status code change #25414

Open jules-ch opened 2 weeks ago

jules-ch commented 2 weeks ago

API V2 Endpoint does not return the correct status code as per the docs. https://docs.influxdata.com/influxdb/v2/api/#operation/PostWrite

Some clients relies on this code not to return an error.

For example :

It's quite easy to change in influxdb_server.http there is already a use_v3 boolean if you want to keep the 200 in v3:

 async fn write_lp_inner(
        &self,
        params: WriteParams,
        req: Request<Body>,
        accept_rp: bool,
        use_v3: bool,
    ) -> Result<Response<Body>> {
        validate_db_name(&params.db, accept_rp)?;
        info!("write_lp to {}", params.db);

        let body = self.read_body(req).await?;
        let body = std::str::from_utf8(&body).map_err(Error::NonUtf8Body)?;

        let database = NamespaceName::new(params.db)?;

        let default_time = self.time_provider.now();

        let result = if use_v3 {
            self.write_buffer
                .write_lp_v3(
                    database,
                    body,
                    default_time,
                    params.accept_partial,
                    params.precision,
                )
                .await?
        } else {
            self.write_buffer
                .write_lp(
                    database,
                    body,
                    default_time,
                    params.accept_partial,
                    params.precision,
                )
                .await?
        };

        if result.invalid_lines.is_empty() {
            if use_v3 {
                Ok(Response::new(Body::empty()))
            } else {
                let response = Response::builder()
                    .status(StatusCode::NO_CONTENT)
                    .body(Body::empty())
                    .unwrap();
                Ok(response)
            }
        } else {
            Err(Error::PartialLpWrite(result))
        }
    }

Steps to reproduce: List the minimal actions needed to reproduce the behaviour.

  1. influxdb3 serve
  2. call V2 endpoint

Expected behaviour: api/v2/write returns 204 status on success

from the docs : https://docs.influxdata.com/influxdb/v2/api/#operation/PostWrite

Actual behaviour: Endpoint returns 200 status code

hiltontj commented 2 weeks ago

Hey @jules-ch - thank you for opening the issue. In that particular case, the use_v3 is deciding whether to use a new experimental line protocol parser, vs. using the existing line protocol parser, so it may not be best to use that for deciding a status code. However, we will certainly need to maintain backward compatibility, so we will have to change things around such that the v2 endpoint returns 204.

It may be that the new v3 endpoint can also return 204, which would make this change even easier, but that hasn't been decided yet. We are actively working towards an initial release and so this code base is still undergoing a lot of changes, but this is an issue we will definitely need to tackle, so thanks again for taking the time to write this up!

pauldix commented 2 weeks ago

The v2 returns a 204 because we built the InfluxDB Cloud 2 as the first v2 product, which accepts data before validating it and making it available for query. Because of that, 204 seemed like the correct response code and we kept that behavior in open source v2 to keep things consistent.

With v3 in both the cloud and open source versions, data is validated and made available for query before a response is sent to the client. So the v3 API endpoint will return a 200, reflecting that behavior. With the v2 compatible API, we should return a 204 to keep things consistent with the previous API version's behavior.

So this is definitely something to fix 😄