jackye1995 commented on a change in pull request #3663:
URL: https://github.com/apache/iceberg/pull/3663#discussion_r775629231



##########
File path: api/src/main/java/org/apache/iceberg/LockManager.java
##########
@@ -17,14 +17,14 @@
  * under the License.
  */
 
-package org.apache.iceberg.aws.glue;
+package org.apache.iceberg;

Review comment:
       yes it can still be in the `util` package. 
   
   I think the actual module and package of this class depends on how we want 
to position this feature. I remember you mentioned we might want to eventually 
introduce some kind of pessimistic lock to fully lock a table to make certain 
operations succeed.
   
   So the following makes sense to me:
   1. if it's only for `HadoopTables` (or maybe `HadoopCatalog`), then 
`iceberg-core` module can work and it can either be in 
`org.apache.iceberg.hadoop` or `org.apache.iceberg.util`.
   2. if we want this to do other things in the future like fully locking a 
table in a catalog, then I think `iceberg-api` module and `org.apache.iceberg` 
package seems more official.




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

Reply via email to