phet commented on code in PR #3849:
URL: https://github.com/apache/gobblin/pull/3849#discussion_r1428325135
##########
gobblin-api/src/main/java/org/apache/gobblin/password/PasswordManager.java:
##########
@@ -124,18 +125,23 @@ private List<TextEncryptor>
getEncryptors(CachedInstanceKey cacheKey) {
suffix = "." + String.valueOf(i);
} catch (FileNotFoundException fnf) {
// It is ok for password files not being present
- LOG.warn("Master password file " + currentMasterPasswordFile + " not
found.");
+ LOG.warn("Master password file '" + currentMasterPasswordFile + "' not
found.");
} catch (IOException ioe) {
exception = ioe;
- LOG.warn("Master password could not be read from file " +
currentMasterPasswordFile);
+ LOG.warn("Master password file could not be read from '" +
currentMasterPasswordFile + "'");
} catch (Exception e) {
- LOG.warn("Encryptor could not be instantiated.");
+ LOG.warn("Encryptor could not be instantiated using file '" +
currentMasterPasswordFile + "'.", e);
}
} while (i++ < numOfEncryptionKeys);
// Throw exception if could not read any existing password file
- if (encryptors.size() < 1 && exception != null) {
- throw new RuntimeException("Master Password could not be read from any
master password file.", exception);
+ if (encryptors.size() < 1) {
+ if (exception != null) {
+ throw new RuntimeException("Master password could not be read from any
master password file.", exception);
+ } else {
+ // TODO: determine whether to always throw whenever no encryptors,
despite `exception == null`! (for now, at least give notice by logging)
Review Comment:
I wavered on this, but ultimately decided not to pursue, as it would NOT be
backwards-compatible semantics and I'm unsure whether any of these situations
are essentially harmless with users happy to continue w/o disruption
--
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]
