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.

Sign up for GitHub

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

fix: correct AppendSerializtionError typo #2037

Merged
merged 9 commits into from
Mar 22, 2023
Merged

fix: correct AppendSerializtionError typo #2037

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
f05a58c
fix: correct AppendSerializtionError typo
Neenu1995 Mar 13, 2023
5c1c803
🦉 Updates from OwlBot post-processor
gcf-owl-bot[bot] Mar 13, 2023
6052442
🦉 Updates from OwlBot post-processor
gcf-owl-bot[bot] Mar 13, 2023
b8940f8
Merge branch 'fix-appendSerializationError' of https://github.com/goo…
gcf-owl-bot[bot] Mar 13, 2023
c237a23
chore: add javadoc
Neenu1995 Mar 14, 2023
ee17683
🦉 Updates from OwlBot post-processor
gcf-owl-bot[bot] Mar 14, 2023
acb10b5
🦉 Updates from OwlBot post-processor
gcf-owl-bot[bot] Mar 14, 2023
227c956
Merge branch 'fix-appendSerializationError' of https://github.com/goo…
gcf-owl-bot[bot] Mar 14, 2023
4cf3817
Merge branch 'main' into fix-appendSerializationError
Neenu1995 Mar 22, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions ...-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1/ConnectionWorker.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
import com.google.api.gax.rpc.FixedHeaderProvider;
import com.google.auto.value.AutoValue;
import com.google.cloud.bigquery.storage.v1.AppendRowsRequest.ProtoData;
import com.google.cloud.bigquery.storage.v1.Exceptions.AppendSerializtionError;
import com.google.cloud.bigquery.storage.v1.Exceptions.AppendSerializationError;
import com.google.cloud.bigquery.storage.v1.StreamConnection.DoneCallback;
import com.google.cloud.bigquery.storage.v1.StreamConnection.RequestCallback;
import com.google.common.annotations.VisibleForTesting;
Expand Down Expand Up @@ -901,8 +901,8 @@ private void requestCallback(AppendRowsResponse response) {
rowIndexToErrorMessage.put(
Math.toIntExact(rowError.getIndex()), rowError.getMessage());
}
AppendSerializtionError exception =
new AppendSerializtionError(
AppendSerializationError exception =
new AppendSerializationError(
response.getError().getCode(),
response.getError().getMessage(),
streamName,
Expand Down
23 changes: 19 additions & 4 deletions ...-cloud-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1/Exceptions.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,10 +216,8 @@ public static StorageException toStorageException(Throwable exception) {
}

/**
* This exception is thrown from {@link JsonStreamWriter#append()} when the client side Json to
* Proto serializtion fails. It can also be thrown by the server in case rows contains invalid
* data. The exception contains a Map of indexes of faulty rows and the corresponding error
* message.
* This class has a typo in the name. It will be removed soon. Please use {@link
* AppendSerializationError}
*/
public static class AppendSerializtionError extends StatusRuntimeException {
Neenu1995 marked this conversation as resolved.
Show resolved Hide resolved
private final Map<Integer, String> rowIndexToErrorMessage;
Expand All @@ -244,6 +242,23 @@ public String getStreamName() {
}
}

/**
* This exception is thrown from {@link JsonStreamWriter#append()} when the client side Json to
* Proto serializtion fails. It can also be thrown by the server in case rows contains invalid
* data. The exception contains a Map of indexes of faulty rows and the corresponding error
* message.
*/
public static class AppendSerializationError extends AppendSerializtionError {
Copy link
Member

Choose a reason for hiding this comment

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

Javadoc? (I expect a copy of AppendSerializtionError) Explanation of typo and backward compatibility is nice.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since we don't want to break customers using the AppendSerializationError(with typo) and at the same time, move all the reference we have to that class in the library, to the class without typo, it made sense to use class AppendSerializationError extends AppendSerializtionError (super class has typo).

The other way round would mean, we move the code in AppendSerializtionError to AppendSerializationError for us to use in the library. That would break the external customers, if we don't move the code, that will cause code duplication.

Copy link
Member

Choose a reason for hiding this comment

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

That makes sense.


public AppendSerializationError(
int codeValue,
String description,
String streamName,
Map<Integer, String> rowIndexToErrorMessage) {
super(codeValue, description, streamName, rowIndexToErrorMessage);
}
}

/** This exception is used internally to handle field level parsing errors. */
public static class FieldParseError extends IllegalArgumentException {
private final String fieldName;
Expand Down
7 changes: 4 additions & 3 deletions ...-bigquerystorage/src/main/java/com/google/cloud/bigquery/storage/v1/JsonStreamWriter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
import com.google.api.gax.core.CredentialsProvider;
import com.google.api.gax.core.ExecutorProvider;
import com.google.api.gax.rpc.TransportChannelProvider;
import com.google.cloud.bigquery.storage.v1.Exceptions.AppendSerializtionError;
import com.google.cloud.bigquery.storage.v1.Exceptions.AppendSerializationError;
import com.google.common.base.Preconditions;
import com.google.protobuf.Descriptors;
import com.google.protobuf.Descriptors.Descriptor;
Expand Down Expand Up @@ -194,7 +194,8 @@ public ApiFuture<AppendRowsResponse> append(JSONArray jsonArr, long offset)
// Any error in convertJsonToProtoMessage will throw an
// IllegalArgumentException/IllegalStateException/NullPointerException.
// IllegalArgumentException will be collected into a Map of row indexes to error messages.
// After the conversion is finished an AppendSerializtionError exception that contains all the
// After the conversion is finished an AppendSerializationError exception that contains all
// the
// conversion errors will be thrown.
long currentRequestSize = 0;
Map<Integer, String> rowIndexToErrorMessage = new HashMap<>();
Expand Down Expand Up @@ -224,7 +225,7 @@ public ApiFuture<AppendRowsResponse> append(JSONArray jsonArr, long offset)
}

if (!rowIndexToErrorMessage.isEmpty()) {
throw new AppendSerializtionError(
throw new AppendSerializationError(
Code.INVALID_ARGUMENT.getNumber(),
"Append serialization failed for writer: " + streamName,
streamName,
Expand Down
18 changes: 9 additions & 9 deletions ...querystorage/src/test/java/com/google/cloud/bigquery/storage/v1/JsonStreamWriterTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
import com.google.cloud.bigquery.storage.test.Test.RepetitionType;
import com.google.cloud.bigquery.storage.test.Test.UpdatedFooType;
import com.google.cloud.bigquery.storage.v1.ConnectionWorkerPool.Settings;
import com.google.cloud.bigquery.storage.v1.Exceptions.AppendSerializtionError;
import com.google.cloud.bigquery.storage.v1.Exceptions.AppendSerializationError;
import com.google.cloud.bigquery.storage.v1.TableFieldSchema.Mode;
import com.google.protobuf.ByteString;
import com.google.protobuf.Descriptors.DescriptorValidationException;
Expand Down Expand Up @@ -1096,7 +1096,7 @@ public void testWithoutIgnoreUnknownFieldsUpdateFail() throws Exception {
try {
ApiFuture<AppendRowsResponse> appendFuture = writer.append(jsonArr);
Assert.fail("expected ExecutionException");
} catch (AppendSerializtionError ex) {
} catch (AppendSerializationError ex) {
assertEquals(
"JSONObject has fields unknown to BigQuery: root.test_unknown.",
ex.getRowIndexToErrorMessage().get(1));
Expand Down Expand Up @@ -1188,7 +1188,7 @@ public void testFlowControlSettingNoLimitBehavior() throws Exception {
}

@Test
public void testMultipleAppendSerializtionErrors()
public void testMultipleAppendSerializationErrors()
throws DescriptorValidationException, IOException, InterruptedException {
FooType expectedProto = FooType.newBuilder().setFoo("allen").build();
JSONObject foo = new JSONObject();
Expand All @@ -1213,10 +1213,10 @@ public void testMultipleAppendSerializtionErrors()
getTestJsonStreamWriterBuilder(TEST_STREAM, TABLE_SCHEMA).build()) {
try {
ApiFuture<AppendRowsResponse> appendFuture = writer.append(jsonArr);
Assert.fail("expected AppendSerializtionError");
} catch (AppendSerializtionError appendSerializtionError) {
Assert.fail("expected AppendSerializationError");
} catch (AppendSerializationError appendSerializationError) {
Map<Integer, String> rowIndexToErrorMessage =
appendSerializtionError.getRowIndexToErrorMessage();
appendSerializationError.getRowIndexToErrorMessage();
assertEquals(2, rowIndexToErrorMessage.size());
assertEquals(
"JSONObject has fields unknown to BigQuery: root.not_foo.",
Expand Down Expand Up @@ -1253,10 +1253,10 @@ public void testBadStringToNumericRowError()
getTestJsonStreamWriterBuilder(TEST_STREAM, TABLE_SCHEMA).build()) {
try {
ApiFuture<AppendRowsResponse> appendFuture = writer.append(jsonArr);
Assert.fail("expected AppendSerializtionError");
} catch (AppendSerializtionError appendSerializtionError) {
Assert.fail("expected AppendSerializationError");
} catch (AppendSerializationError appendSerializationError) {
Map<Integer, String> rowIndexToErrorMessage =
appendSerializtionError.getRowIndexToErrorMessage();
appendSerializationError.getRowIndexToErrorMessage();
assertEquals(1, rowIndexToErrorMessage.size());
assertTrue(
rowIndexToErrorMessage
Expand Down
6 changes: 3 additions & 3 deletions ...rc/test/java/com/google/cloud/bigquery/storage/v1/it/ITBigQueryWriteManualClientTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
import com.google.cloud.bigquery.Schema;
import com.google.cloud.bigquery.storage.test.Test.*;
import com.google.cloud.bigquery.storage.v1.*;
import com.google.cloud.bigquery.storage.v1.Exceptions.AppendSerializtionError;
import com.google.cloud.bigquery.storage.v1.Exceptions.AppendSerializationError;
import com.google.cloud.bigquery.storage.v1.Exceptions.OffsetAlreadyExists;
import com.google.cloud.bigquery.storage.v1.Exceptions.OffsetOutOfRange;
import com.google.cloud.bigquery.storage.v1.Exceptions.SchemaMismatchedException;
Expand Down Expand Up @@ -372,8 +372,8 @@ public void testRowErrors()
} catch (Throwable t) {
assertTrue(t instanceof ExecutionException);
t = t.getCause();
assertTrue(t instanceof AppendSerializtionError);
AppendSerializtionError e = (AppendSerializtionError) t;
assertTrue(t instanceof AppendSerializationError);
AppendSerializationError e = (AppendSerializationError) t;
LOG.info("Found row errors on stream: " + e.getStreamName());
assertEquals(
"Field foo: STRING(10) has maximum length 10 but got a value with length 12 on field foo.",
Expand Down
6 changes: 3 additions & 3 deletions samples/snippets/src/main/java/com/example/bigquerystorage/WriteToDefaultStream.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
import com.google.cloud.bigquery.storage.v1.AppendRowsResponse;
import com.google.cloud.bigquery.storage.v1.BigQueryWriteClient;
import com.google.cloud.bigquery.storage.v1.Exceptions;
import com.google.cloud.bigquery.storage.v1.Exceptions.AppendSerializtionError;
import com.google.cloud.bigquery.storage.v1.Exceptions.AppendSerializationError;
import com.google.cloud.bigquery.storage.v1.Exceptions.StorageException;
import com.google.cloud.bigquery.storage.v1.JsonStreamWriter;
import com.google.cloud.bigquery.storage.v1.TableName;
Expand Down Expand Up @@ -218,8 +218,8 @@ public void onFailure(Throwable throwable) {
}
}

if (throwable instanceof AppendSerializtionError) {
AppendSerializtionError ase = (AppendSerializtionError) throwable;
if (throwable instanceof AppendSerializationError) {
AppendSerializationError ase = (AppendSerializationError) throwable;
Map<Integer, String> rowIndexToErrorMessage = ase.getRowIndexToErrorMessage();
if (rowIndexToErrorMessage.size() > 0) {
// Omit the faulty rows
Expand Down