You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by GitBox <gi...@apache.org> on 2022/05/30 02:37:11 UTC
[GitHub] [iceberg] bryanck opened a new pull request, #4897: Minor adjustments to catalog tests for flexibility
bryanck opened a new pull request, #4897:
URL: https://github.com/apache/iceberg/pull/4897
This PR makes a couple of minor changes to make the catalog tests a little bit more flexible. For example, it makes some list checks ignore element order.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on a diff in pull request #4897: Minor adjustments to catalog tests for flexibility
Posted by GitBox <gi...@apache.org>.
rdblue commented on code in PR #4897:
URL: https://github.com/apache/iceberg/pull/4897#discussion_r884905436
##########
core/src/test/java/org/apache/iceberg/catalog/CatalogTests.java:
##########
@@ -163,7 +162,7 @@ public void testCreateNamespace() {
Assert.assertFalse("Namespace should not exist", catalog.namespaceExists(NS));
catalog.createNamespace(NS);
- Assert.assertEquals("Catalog should have the created namespace", ImmutableList.of(NS), catalog.listNamespaces());
+ Assert.assertTrue("Catalog should have the created namespace", catalog.listNamespaces().contains(NS));
Review Comment:
Yeah, good call here. It's fine if the catalog creates default namespaces.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on pull request #4897: Minor adjustments to catalog tests for flexibility
Posted by GitBox <gi...@apache.org>.
rdblue commented on PR #4897:
URL: https://github.com/apache/iceberg/pull/4897#issuecomment-1141244488
Thanks, @bryanck! These changes look good to me and should help people get tests passing.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] bryanck commented on a diff in pull request #4897: Minor adjustments to catalog tests for flexibility
Posted by GitBox <gi...@apache.org>.
bryanck commented on code in PR #4897:
URL: https://github.com/apache/iceberg/pull/4897#discussion_r884379240
##########
core/src/main/java/org/apache/iceberg/rest/CatalogHandlers.java:
##########
@@ -279,9 +279,10 @@ private static boolean isCreate(UpdateTableRequest request) {
}
private static TableMetadata create(TableOperations ops, TableMetadata start, UpdateTableRequest request) {
- TableMetadata.Builder builder = TableMetadata.buildFrom(start);
-
// the only valid requirement is that the table will be created
+ request.requirements().forEach(requirement -> requirement.validate(ops.current()));
Review Comment:
I added this so the assertion for validation check exception message is consistent with other validation checks.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue merged pull request #4897: Minor adjustments to catalog tests for flexibility
Posted by GitBox <gi...@apache.org>.
rdblue merged PR #4897:
URL: https://github.com/apache/iceberg/pull/4897
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org