JonasJ-ap commented on code in PR #5797:
URL: https://github.com/apache/iceberg/pull/5797#discussion_r974836109
##########
aws/src/main/java/org/apache/iceberg/aws/glue/GlueCatalog.java:
##########
@@ -228,13 +228,7 @@ protected TableOperations newTableOps(TableIdentifier
tableIdentifier) {
}
return new GlueTableOperations(
- glue,
- lockManager,
- catalogName,
- awsProperties,
- catalogProperties,
Review Comment:
Thank you so much for your review and suggestions. Now I see that there are
some integration tests using the wrong `initialize()` call and thus skip
initializing this particular field. I will update the changes to those tests in
#5784.
Meanwhile, I am curious if the return statement here is worth a change.
Given that this return statement always receives null for `catalogProperties`
and methods in `GlueTableOperations` do not fully handle the `null` case, it
seems not a good idea to initialize `GlueTableOperations` in this way. Maybe we
could still call `properties()` here or delete this return statement since it
is unreachable under the current code logic? I appreciate your time and help.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
