fix: send correct time unit when writing Points#384
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #384 +/- ##
==========================================
+ Coverage 88.19% 88.25% +0.05%
==========================================
Files 21 21
Lines 1525 1532 +7
Branches 275 277 +2
==========================================
+ Hits 1345 1352 +7
Misses 82 82
Partials 98 98 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR addresses a write precision mismatch when writing Point objects to InfluxDB v3: the line protocol payload is serialized with nanosecond timestamps, so the request’s precision query parameter must match to avoid server-side “timestamp out of range for precision” errors (issue #382).
Changes:
- Force
precision=nanosecondin write query parameters when writingPointinstances. - Update unit tests to expect
precision=nanosecondforPointwrites even when client config sets another precision. - Add an end-to-end integration test covering writing points with timestamps provided in different input units.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/main/java/com/influxdb/v3/client/internal/InfluxDBClientImpl.java | Changes precision selection logic for write requests when writing Points. |
| src/test/java/com/influxdb/v3/client/InfluxDBClientWriteTest.java | Updates expected precision in write request assertions for point writes. |
| src/test/java/com/influxdb/v3/client/integration/E2ETest.java | Adds an integration test that writes multiple points with mixed timestamp input units. |
| CHANGELOG.md | Documents the bug fix in the unreleased section. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
e58adf7 to
7cebe1b
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
src/main/java/com/influxdb/v3/client/internal/InfluxDBClientImpl.java:34
- Unused import
java.util.function.Predicateis introduced but not used anywhere in this class. This will fail the repository's CheckstyleUnusedImportsrule; please remove it.
import java.util.logging.Logger;
3a68973 to
434f7a9
Compare
d0d491b to
e180f32
Compare
alespour
left a comment
There was a problem hiding this comment.
I think we should tweak javadoc for precision args just a tiny bit, Copilot finding makes sense. And cover the logic more by unit test in addition to e2e test.
|
|
||
| checkWriteCalled("/api/v3/write_lp", "DB", expectedPrecision, true, "true", null, true); | ||
| } | ||
|
|
There was a problem hiding this comment.
This should improve coverage of fixed logic by unit tests.
| @ParameterizedTest(name = "{0}") | |
| @MethodSource("pointPrecisionIgnoredCases") | |
| void pointWritesIgnoreWriteOptionsPrecision( | |
| String name, | |
| WritePrecision configuredPrecision, | |
| boolean manyPoints) throws Exception { | |
| mockServer.enqueue(createResponse(200)); | |
| ClientConfig cfg = new ClientConfig.Builder() | |
| .host(baseURL) | |
| .token("TOKEN".toCharArray()) | |
| .database("DB") | |
| .build(); | |
| try (InfluxDBClient client = InfluxDBClient.getInstance(cfg)) { | |
| Point point = new Point("mem"); | |
| point.setTag("tag", "one"); | |
| point.setField("value", 1.0); | |
| WriteOptions options = new WriteOptions.Builder() | |
| .precision(configuredPrecision) | |
| .build(); | |
| if (manyPoints) { | |
| client.writePoints(List.of(point), options); | |
| } else { | |
| client.writePoint(point, options); | |
| } | |
| } | |
| checkWriteCalled("/api/v3/write_lp", "DB", "nanosecond", true, null, null, false); | |
| } | |
| private static Stream<Arguments> pointPrecisionIgnoredCases() { | |
| return Stream.of( | |
| Arguments.of("writePoint precision=S", WritePrecision.S, false), | |
| Arguments.of("writePoint precision=MS", WritePrecision.MS, false), | |
| Arguments.of("writePoints precision=S", WritePrecision.S, true), | |
| Arguments.of("writePoints precision=US", WritePrecision.US, true) | |
| ); | |
| } | |
| * <li><code>organization</code> - organization to be used for operations</li> | ||
| * <li><code>database</code> - database to be used for InfluxDB operations</li> | ||
| * <li><code>writePrecision</code> - precision to use when writing points to InfluxDB</li> | ||
| * <li><code>writePrecision</code> - precision to use when writing points to InfluxDB. |
There was a problem hiding this comment.
I would just remove a word to avoid a bit confusing contradiction with the sentence that follows after it.
| * <li><code>writePrecision</code> - precision to use when writing points to InfluxDB. | |
| * <li><code>writePrecision</code> - precision to use when writing to InfluxDB. |
| @@ -548,7 +551,10 @@ public Builder database(@Nullable final String database) { | |||
| * if no precision is specified in the write API call. | |||
| * | |||
| * @param writePrecision default precision to use for the timestamp of points | |||
There was a problem hiding this comment.
I would just remove a word to avoid a bit confusing contradiction with the sentence that follows after it.
| * @param writePrecision default precision to use for the timestamp of points | |
| * @param writePrecision default precision to use for the timestamp |
| @@ -140,6 +145,9 @@ public WriteOptions(@Nullable final String database, | |||
| * If it is not specified then use {@link ClientConfig#getDatabase()}. | |||
| * @param precision The precision to use for the timestamp of points. | |||
There was a problem hiding this comment.
I would just remove a word to avoid a bit confusing contradiction with the sentence that follows after it.
| * @param precision The precision to use for the timestamp of points. | |
| * @param precision The precision to use for the timestamp. |
| @@ -158,6 +166,9 @@ public WriteOptions(@Nullable final String database, | |||
| * If it is not specified then use {@link ClientConfig#getDatabase()}. | |||
| * @param precision The precision to use for the timestamp of points. | |||
There was a problem hiding this comment.
| * @param precision The precision to use for the timestamp of points. | |
| * @param precision The precision to use for the timestamp. |
| @@ -187,6 +198,9 @@ public WriteOptions(@Nullable final Map<String, String> headers) { | |||
| * If it is not specified then use {@link ClientConfig#getDatabase()}. | |||
| * @param precision The precision to use for the timestamp of points. | |||
There was a problem hiding this comment.
| * @param precision The precision to use for the timestamp of points. | |
| * @param precision The precision to use for the timestamp. |
| @@ -209,6 +223,9 @@ public WriteOptions(@Nullable final String database, | |||
| * If it is not specified then use {@link ClientConfig#getDatabase()}. | |||
| * @param precision The precision to use for the timestamp of points. | |||
There was a problem hiding this comment.
| * @param precision The precision to use for the timestamp of points. | |
| * @param precision The precision to use for the timestamp. |
| @@ -234,6 +251,9 @@ public WriteOptions(@Nullable final String database, | |||
| * If it is not specified then use {@link ClientConfig#getDatabase()}. | |||
| * @param precision The precision to use for the timestamp of points. | |||
There was a problem hiding this comment.
| * @param precision The precision to use for the timestamp of points. | |
| * @param precision The precision to use for the timestamp. |
| @@ -265,6 +285,9 @@ public WriteOptions(@Nullable final String database, | |||
| * If it is not specified then use {@link ClientConfig#getDatabase()}. | |||
| * @param precision The precision to use for the timestamp of points. | |||
There was a problem hiding this comment.
| * @param precision The precision to use for the timestamp of points. | |
| * @param precision The precision to use for the timestamp. |
| @@ -307,6 +330,9 @@ public WriteOptions(@Nullable final String database, | |||
| * If it is not specified then use {@link ClientConfig#getDatabase()}. | |||
| * @param precision The precision to use for the timestamp of points. | |||
There was a problem hiding this comment.
| * @param precision The precision to use for the timestamp of points. | |
| * @param precision The precision to use for the timestamp. |
Closes #382
Proposed Changes
precisionin query parameters always set tonanosecondbecause time in Points always converted intonanosecondsby default.Checklist