danielcweeks commented on pull request #4054: URL: https://github.com/apache/iceberg/pull/4054#issuecomment-1033136607
Hey @mayursrivastava, took a quick look and overall, I think this approach will work for testing purposes. A couple observations: 1. I'm not sure if we want to include this in the core library (I think @kbendick made the same observation). One option might actually be to put this implementation into the test src so that there isn't confusion around the fact that this is intended for for test use only (i.e. It's not a supported Catalog/FileIO for production use). Projects that want to use this implementation just need to depend on the test artifact to get access (a good example is the `iceberg-data` project). 2. Even though we have concurrent collections, there are a fair number of places where we're doing checks/changes without synchronizing on the collection, so there are race conditions (I pointed out the `renameTable`, but there are many others and I believe even the table ops commit is not atomic. -- 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]
