Copilot commented on code in PR #8426:
URL: https://github.com/apache/hbase/pull/8426#discussion_r3488118695
##########
hbase-common/src/test/java/org/apache/hadoop/hbase/io/crypto/tls/X509TestContext.java:
##########
@@ -187,85 +188,68 @@ public File getTrustStoreFile(KeyStoreFileType
storeFileType) throws IOException
}
}
- private File getTrustStoreJksFile() throws IOException {
+ private void atomicWriteFile(File file, byte[] content) throws IOException {
+ File tmpFile = new File(file.getParentFile(),
file.getName().concat(".tmp"));
+ Files.write(tmpFile.toPath(), content, StandardOpenOption.CREATE,
+ StandardOpenOption.TRUNCATE_EXISTING);
+ Files.move(tmpFile.toPath(), file.toPath(),
StandardCopyOption.REPLACE_EXISTING,
+ StandardCopyOption.ATOMIC_MOVE);
+ }
Review Comment:
atomicWriteFile uses StandardCopyOption.ATOMIC_MOVE unconditionally.
ATOMIC_MOVE is not guaranteed to be supported by all file systems, so this can
throw AtomicMoveNotSupportedException and make tests flaky on some CI
environments. Consider falling back to a non-atomic move when atomic move is
unsupported (keeping REPLACE_EXISTING).
##########
hbase-common/src/test/java/org/apache/hadoop/hbase/io/crypto/tls/X509TestContext.java:
##########
@@ -171,7 +172,7 @@ public char[] getTrustStorePassword() {
* @return the path to the trust store file.
* @throws IOException if there is an error creating the trust store file.
Review Comment:
The Javadoc for getTrustStoreFile says the temp file "will be deleted on
exit" and documents "@throws IOException", but the implementation no longer
registers deleteOnExit and the method signature now throws Exception. Please
update the Javadoc to match the current lifecycle/throws behavior.
##########
hbase-common/src/test/java/org/apache/hadoop/hbase/io/crypto/tls/X509TestContext.java:
##########
@@ -292,7 +276,7 @@ public Configuration getConf() {
* @return the path to the key store file.
Review Comment:
The Javadoc for getKeyStoreFile still says the temp file "will be deleted on
exit" and documents "@throws IOException", but the method signature now throws
Exception and deleteOnExit is no longer used. Please update the Javadoc so it
matches the current behavior.
--
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]
