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/11/21 17:48:43 UTC

[GitHub] [iceberg] ajantha-bhat opened a new pull request, #6240: Nessie: Refactor NessieTableOperations#doCommit

ajantha-bhat opened a new pull request, #6240:
URL: https://github.com/apache/iceberg/pull/6240

   Move core logic from `NessieTableOperations#doCommit` to `NessieIcebergClient#commitTable` because Trino Nessie catalog integration (https://github.com/trinodb/trino/pull/11701) don't use Iceberg's `NessieTableOperation` directly. 
   Hence to avoid code duplication, move common logic to `NessieIcebergClient`


-- 
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] ajantha-bhat commented on pull request #6240: Nessie: Refactor NessieTableOperations#doCommit

Posted by GitBox <gi...@apache.org>.
ajantha-bhat commented on PR #6240:
URL: https://github.com/apache/iceberg/pull/6240#issuecomment-1322441438

   cc: @nastra 


-- 
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] ajantha-bhat commented on pull request #6240: Nessie: Refactor NessieTableOperations#doCommit

Posted by GitBox <gi...@apache.org>.
ajantha-bhat commented on PR #6240:
URL: https://github.com/apache/iceberg/pull/6240#issuecomment-1323499398

   javadoc task Build failed due to a server timeout error! Will retrigger the CI by rebasing.
   
   ```
   2022-11-22T10:36:40.1649273Z Starting a Gradle Daemon (subsequent builds will be faster)
   2022-11-22T10:41:07.6675520Z 
   2022-11-22T10:41:07.6676645Z FAILURE: Build failed with an exception.
   2022-11-22T10:41:07.6677954Z 
   2022-11-22T10:41:07.6678354Z * What went wrong:
   2022-11-22T10:41:07.6680634Z A problem occurred configuring root project 'iceberg'.
   2022-11-22T10:41:07.6681803Z > Could not resolve all files for configuration ':classpath'.
   2022-11-22T10:41:07.6684778Z    > Could not download gradle-revapi-1.7.0.jar (com.palantir.gradle.revapi:gradle-revapi:1.7.0)
   2022-11-22T10:41:07.6699415Z       > Could not get resource 'https://plugins.gradle.org/m2/com/palantir/gradle/revapi/gradle-revapi/1.7.0/gradle-revapi-1.7.0.jar'.
   2022-11-22T10:41:07.6699795Z 
   2022-11-22T10:41:07.6701580Z          > Could not GET 'https://plugins-artifacts.gradle.org/com.palantir.gradle.revapi/gradle-revapi/1.7.0/e2ab3a8518ad6a424a09a5e605b8f81c998df292eae02707aa4c81a022d59d05/gradle-revapi-1.7.0.jar'.
   2022-11-22T10:41:07.6702429Z Deprecated Gradle features were used in this build, making it incompatible with Gradle 8.0.
   2022-11-22T10:41:07.6703638Z             > Read timed out
   ```


-- 
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] ajantha-bhat commented on pull request #6240: Nessie: Refactor NessieTableOperations#doCommit

Posted by GitBox <gi...@apache.org>.
ajantha-bhat commented on PR #6240:
URL: https://github.com/apache/iceberg/pull/6240#issuecomment-1323253673

   @nastra: Review from Iceberg side is easy. But I would like to know, can Trino really use it?
   Looks to me that `client#commitTable` needs the below parameters. 
   
   ```
   commitTable(
         TableMetadata base,
         TableMetadata metadata,
         String newMetadataLocation,
         IcebergTable expectedContent,
         ContentKey expectedContent)
   ```
   
   But the Trino interface (`NessieIcebergTableOperations#commitNewTable`) currently doesn't have `base`, `expectedContent` and these interfaces are common for other catalogs too. 


-- 
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] nastra commented on a diff in pull request #6240: Nessie: Refactor NessieTableOperations#doCommit

Posted by GitBox <gi...@apache.org>.
nastra commented on code in PR #6240:
URL: https://github.com/apache/iceberg/pull/6240#discussion_r1028365461


##########
nessie/src/main/java/org/apache/iceberg/nessie/NessieIcebergClient.java:
##########
@@ -46,15 +50,7 @@
 import org.projectnessie.error.NessieNamespaceNotFoundException;
 import org.projectnessie.error.NessieNotFoundException;
 import org.projectnessie.error.NessieReferenceNotFoundException;
-import org.projectnessie.model.Branch;
-import org.projectnessie.model.Content;
-import org.projectnessie.model.ContentKey;
-import org.projectnessie.model.EntriesResponse;
-import org.projectnessie.model.GetNamespacesResponse;
-import org.projectnessie.model.IcebergTable;
-import org.projectnessie.model.Operation;
-import org.projectnessie.model.Reference;
-import org.projectnessie.model.Tag;
+import org.projectnessie.model.*;

Review Comment:
   I don't think we allow * imports in the project



-- 
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] nastra commented on pull request #6240: Nessie: Refactor NessieTableOperations#doCommit

Posted by GitBox <gi...@apache.org>.
nastra commented on PR #6240:
URL: https://github.com/apache/iceberg/pull/6240#issuecomment-1323227993

   cc: @dimas-b / @snazy can you guys review this one please?
   


-- 
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] ajantha-bhat commented on a diff in pull request #6240: Nessie: Refactor NessieTableOperations#doCommit

Posted by GitBox <gi...@apache.org>.
ajantha-bhat commented on code in PR #6240:
URL: https://github.com/apache/iceberg/pull/6240#discussion_r1028371605


##########
nessie/src/main/java/org/apache/iceberg/nessie/NessieIcebergClient.java:
##########
@@ -46,15 +50,7 @@
 import org.projectnessie.error.NessieNamespaceNotFoundException;
 import org.projectnessie.error.NessieNotFoundException;
 import org.projectnessie.error.NessieReferenceNotFoundException;
-import org.projectnessie.model.Branch;
-import org.projectnessie.model.Content;
-import org.projectnessie.model.ContentKey;
-import org.projectnessie.model.EntriesResponse;
-import org.projectnessie.model.GetNamespacesResponse;
-import org.projectnessie.model.IcebergTable;
-import org.projectnessie.model.Operation;
-import org.projectnessie.model.Reference;
-import org.projectnessie.model.Tag;
+import org.projectnessie.model.*;

Review Comment:
   It is draft PR. My bad. Let me clean it up.



-- 
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] Fokko merged pull request #6240: Nessie: Refactor NessieTableOperations#doCommit

Posted by GitBox <gi...@apache.org>.
Fokko merged PR #6240:
URL: https://github.com/apache/iceberg/pull/6240


-- 
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] nastra commented on pull request #6240: Nessie: Refactor NessieTableOperations#doCommit

Posted by GitBox <gi...@apache.org>.
nastra commented on PR #6240:
URL: https://github.com/apache/iceberg/pull/6240#issuecomment-1323427139

   I've verified that this new API change works for https://github.com/trinodb/trino/pull/11701


-- 
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