feat: introduce java.time
#2415
Conversation
product-auto-label
bot
added
size: l
…ava-bigtable into introduce-java-time
…ds + nanos)" This reverts commit a7746ad.
diegomarquezp
added
the
kokoro:force-run
yoshi-kokoro
removed
the
kokoro:force-run
| @@ -23,8 +23,8 @@ | |||
| import com.google.common.base.Objects; | |||
| import com.google.common.collect.ImmutableList; | |||
| import com.google.protobuf.ByteString; | |||
| import java.time.Instant; | |||
| @@ -150,7 +149,7 @@ static SqlType<Boolean> bool() { | |||
| } | |||
|
|
|||
| /** returns a {@link SqlType} for the {@code TIMESTAMP} type. */ | |||
| static SqlType<Instant> timestamp() { | |||
| static SqlType<java.time.Instant> timestamp() { | |||
There was a problem hiding this comment.
Types.Timestamp is @InternalApi
| @@ -239,7 +249,7 @@ private static Value booleanParamOf(@Nullable Boolean value) { | |||
| return builder.build(); | |||
| } | |||
|
|
|||
| private static Value timestampParamOf(@Nullable Instant value) { | |||
| private static Value timestampParamOf(@Nullable java.time.Instant value) { | |||
There was a problem hiding this comment.
Timestamp is @InternalApi
| @@ -30,13 +30,13 @@ | |||
| import com.google.common.base.Preconditions; | |||
| import com.google.common.base.Stopwatch; | |||
| import com.google.common.util.concurrent.RateLimiter; | |||
| import java.time.Duration; | |||
| import java.time.Instant; | |||
There was a problem hiding this comment.
this class is package private
...able/src/main/java/com/google/cloud/bigtable/data/v2/internal/AbstractProtoStructReader.java
Show resolved
Hide resolved
...able/src/main/java/com/google/cloud/bigtable/data/v2/internal/AbstractProtoStructReader.java
Show resolved
Hide resolved
...e-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/internal/ResultSetImpl.java
Outdated
Show resolved
Hide resolved
...src/main/java/com/google/cloud/bigtable/data/v2/models/DefaultChangeStreamRecordAdapter.java
Outdated
Show resolved
Hide resolved
...src/main/java/com/google/cloud/bigtable/data/v2/models/DefaultChangeStreamRecordAdapter.java
Show resolved
Hide resolved
| } | ||
|
|
||
| /** Sets the heartbeat duration for the change stream. */ | ||
| public ReadChangeStreamQuery heartbeatDurationDuration(java.time.Duration duration) { |
There was a problem hiding this comment.
This is durationduration again
There was a problem hiding this comment.
Changed to heartbeatDurationJavaTime in the meantime
There was a problem hiding this comment.
Let's wait for googleapis/java-datastore#1671 (comment)
There was a problem hiding this comment.
@igorbernstein2 we still haven't decided about how to rename this method. So far we have heartBeatDurationJavaTime as it would keep the prefix.
...-bigtable/src/main/java/com/google/cloud/bigtable/gaxx/retrying/RetryInfoRetryAlgorithm.java
Show resolved
Hide resolved
| // From java 15, now() provides nanosecond precision. We trim micros and nanos, so it matches | ||
| // bigtable's millisecond precision api. | ||
| // see https://bugs.openjdk.org/browse/JDK-8242504 |
There was a problem hiding this comment.
Thanks for calling this out. Let's make sure Bigtable is aware of this.
Let's keep it as ms precision for this PR and they can choose to keep it or update it to nanosecond in the future.
There was a problem hiding this comment.
@igorbernstein2 FYI
| @@ -100,10 +102,10 @@ public ChangeStreamRecord onCloseStream(ReadChangeStreamResponse.CloseStream clo | |||
|
|
|||
| /** {@inheritDoc} */ | |||
| @Override | |||
| public void startUserMutation( | |||
| public void startUserMutationInstant( | |||
There was a problem hiding this comment.
I know this is marked with @InternalApi. Let's double check that this is fine (hopefully no BigTable users are using internalApi marked apis)
There was a problem hiding this comment.
@igorbernstein2 is it ok to change the signature of this method directly since it's marked as @InternalApi?
This PR introduces
java.timealternatives to existingorg.threeten.bp.*methods, as well as switching internal variables (if any) tojava.timeThe main constraint is to keep the changes backwards compatible, so for each existing threeten method "
method1(org.threeten.bp.Duration)" we will add an alternative with a Duration (or Timestamp when applicable) suffix: "method1Duration(java.time.Duration)".For most cases, the implementation will be held in the
java.timemethod and the old threeten method will just delegate the call to it. However, for the case of abstract classes, the implementation will be kept in the threeten method to avoid breaking changes (i.e. users that already overloaded the method in their user code).Note: https://cloud.google.com/bigtable/docs/reference/sql/data-types#timestamp_type implies that nanosecond precision will be ignored.