amogh-jahagirdar commented on code in PR #9498:
URL: https://github.com/apache/iceberg/pull/9498#discussion_r1456088754
##########
core/src/main/java/org/apache/iceberg/hadoop/HadoopTableOperations.java:
##########
@@ -360,7 +360,10 @@ int findVersion() {
*/
private void renameToFinal(FileSystem fs, Path src, Path dst, int
nextVersion) {
try {
- lockManager.acquire(dst.toString(), src.toString());
+ boolean success = lockManager.acquire(dst.toString(), src.toString());
+ if (!success) {
+ throw new CommitFailedException("Failed to acquire lock on file: %s",
dst);
+ }
Review Comment:
Style nit, if we could add a new line after this `if` block separating it
from the next `if`. I know in this file it looks like we didn't really follow
our normal practice already but at least for new changes we could follow it.
Also I think I'd just inline the boolean so
```
if (!lockManager.acquire(dst.toString(), src.toStrnig()) {
throw ...
}
```
--
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-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
