mmoayyed commented on a change in pull request #178:
URL: https://github.com/apache/syncope/pull/178#discussion_r417290694
##########
File path:
wa/starter/src/main/java/org/apache/syncope/wa/SyncopeCoreNotReadyException.java
##########
@@ -0,0 +1,25 @@
+/*
Review comment:
Strictly speaking, this is not required here because we'd be making an
assumption that core is in fact ready by the time this method is called (and is
only called once the app is ready). If by then core is not ready, we should
likely consider that a catastrophic event and end the app, specially because
this (metadata, etc) is not the sort of thing one can ignore unlike other
places. So perhaps we can just return `false` here, but...
..more importantly, I see the catch clause that traps `Exception`, and by
extension this one, just logs a warning. I don't think that's right; at the
point in the lifetime of the app when this method is called, core must be
ready. Its unavailability should halt the app and present an error (more than a
warning).
##########
File path:
wa/starter/src/main/java/org/apache/syncope/wa/SyncopeCoreNotReadyException.java
##########
@@ -0,0 +1,25 @@
+/*
Review comment:
Strictly speaking, this is not required here because we'd be making an
assumption that core is in fact ready by the time this method is called (and is
only called once the app is ready). If by then core is not ready, we should
likely consider that a catastrophic event and end the app, specially because
this (metadata, etc) is not the sort of thing one can ignore unlike other
places. So perhaps we can just return `false` here, but...
...more importantly, I see the catch clause that traps `Exception`, and by
extension this one, just logs a warning. I don't think that's right; at the
point in the lifetime of the app when this method is called, core must be
ready. Its unavailability should halt the app and present an error (more than a
warning).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
