You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by GitBox <gi...@apache.org> on 2021/10/22 10:43:34 UTC
[GitHub] [ignite-3] AMashenkov opened a new pull request #408: IGNITE-15754 Fix marshaller exception handling.
AMashenkov opened a new pull request #408:
URL: https://github.com/apache/ignite-3/pull/408
https://issues.apache.org/jira/browse/IGNITE-15754
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] ygerzhedovich commented on a change in pull request #408: IGNITE-15754: Fix marshaller exception handling
Posted by GitBox <gi...@apache.org>.
ygerzhedovich commented on a change in pull request #408:
URL: https://github.com/apache/ignite-3/pull/408#discussion_r740160094
##########
File path: modules/table/src/main/java/org/apache/ignite/internal/table/RecordBinaryViewImpl.java
##########
@@ -304,7 +305,7 @@ public RecordBinaryViewImpl(InternalTable tbl, SchemaRegistry schemaReg, TableMa
HashSet<BinaryRow> keys = new HashSet<>(recs.size());
for (Tuple keyRec : recs) {
- final Row keyRow = marshaller().marshal(keyRec);
+ final Row keyRow = marshal(keyRec, false);
Review comment:
Are you sure that should be false? the method name is 'deleteAllExactAsync' so we should check not only key
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] AMashenkov merged pull request #408: IGNITE-15754: Fix marshaller exception handling
Posted by GitBox <gi...@apache.org>.
AMashenkov merged pull request #408:
URL: https://github.com/apache/ignite-3/pull/408
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] ygerzhedovich commented on a change in pull request #408: IGNITE-15754: Fix marshaller exception handling
Posted by GitBox <gi...@apache.org>.
ygerzhedovich commented on a change in pull request #408:
URL: https://github.com/apache/ignite-3/pull/408#discussion_r740160094
##########
File path: modules/table/src/main/java/org/apache/ignite/internal/table/RecordBinaryViewImpl.java
##########
@@ -304,7 +305,7 @@ public RecordBinaryViewImpl(InternalTable tbl, SchemaRegistry schemaReg, TableMa
HashSet<BinaryRow> keys = new HashSet<>(recs.size());
for (Tuple keyRec : recs) {
- final Row keyRow = marshaller().marshal(keyRec);
+ final Row keyRow = marshal(keyRec, false);
Review comment:
Are you sure that should be false? the method name is 'deleteAllExactAsync' so we should check not only key
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] ygerzhedovich commented on a change in pull request #408: IGNITE-15754: Fix marshaller exception handling
Posted by GitBox <gi...@apache.org>.
ygerzhedovich commented on a change in pull request #408:
URL: https://github.com/apache/ignite-3/pull/408#discussion_r740167626
##########
File path: modules/table/src/test/java/org/apache/ignite/internal/table/RecordBinaryViewOperationsTest.java
##########
@@ -410,4 +413,17 @@ void assertEqualsValues(SchemaDescriptor schema, Tuple expected, Tuple actual) {
@NotNull private TableImpl createTableImpl(SchemaDescriptor schema) {
return new TableImpl(new DummyInternalTableImpl(), new DummySchemaManagerImpl(schema), null);
}
+
+ public <T extends Throwable> void assertThrowsWithCause(Class<T> expectedType, Executable executable) {
Review comment:
maybe it's time to introduce some TestUtils. WDYT?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] ygerzhedovich commented on a change in pull request #408: IGNITE-15754: Fix marshaller exception handling
Posted by GitBox <gi...@apache.org>.
ygerzhedovich commented on a change in pull request #408:
URL: https://github.com/apache/ignite-3/pull/408#discussion_r740158405
##########
File path: modules/table/src/main/java/org/apache/ignite/internal/table/RecordBinaryViewImpl.java
##########
@@ -265,7 +266,7 @@ public RecordBinaryViewImpl(InternalTable tbl, SchemaRegistry schemaReg, TableMa
@Override public @NotNull CompletableFuture<Tuple> getAndDeleteAsync(@NotNull Tuple rec) {
Objects.requireNonNull(rec);
- final Row row = marshaller().marshalKey(rec);
+ final Row row = marshal(rec, true);
Review comment:
```suggestion
final Row keyRow = marshal(rec, true);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] AMashenkov commented on a change in pull request #408: IGNITE-15754: Fix marshaller exception handling
Posted by GitBox <gi...@apache.org>.
AMashenkov commented on a change in pull request #408:
URL: https://github.com/apache/ignite-3/pull/408#discussion_r740181595
##########
File path: modules/table/src/test/java/org/apache/ignite/internal/table/RecordBinaryViewOperationsTest.java
##########
@@ -410,4 +413,17 @@ void assertEqualsValues(SchemaDescriptor schema, Tuple expected, Tuple actual) {
@NotNull private TableImpl createTableImpl(SchemaDescriptor schema) {
return new TableImpl(new DummyInternalTableImpl(), new DummySchemaManagerImpl(schema), null);
}
+
+ public <T extends Throwable> void assertThrowsWithCause(Class<T> expectedType, Executable executable) {
Review comment:
I think we should check a public exception here and error code, once we will introduce codes and public exceptions.
So, an assertThrowsWithCause must be replaced to junit assertThrows with a correct public exception.
We check a cause here, which is an internal exception, as a temporal solution unless we have public classes and error codes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] ygerzhedovich commented on a change in pull request #408: IGNITE-15754: Fix marshaller exception handling
Posted by GitBox <gi...@apache.org>.
ygerzhedovich commented on a change in pull request #408:
URL: https://github.com/apache/ignite-3/pull/408#discussion_r740164003
##########
File path: modules/table/src/main/java/org/apache/ignite/internal/table/RecordBinaryViewImpl.java
##########
@@ -111,7 +112,7 @@ public RecordBinaryViewImpl(InternalTable tbl, SchemaRegistry schemaReg, TableMa
@Override public @NotNull CompletableFuture<Void> upsertAsync(@NotNull Tuple rec) {
Objects.requireNonNull(rec);
- final Row keyRow = marshaller().marshal(rec);
+ final Row keyRow = marshal(rec, false);
Review comment:
```suggestion
final Row row = marshal(rec, false);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org