Will-Lo commented on code in PR #3551:
URL: https://github.com/apache/gobblin/pull/3551#discussion_r965118960
##########
gobblin-modules/gobblin-azkaban/src/main/java/org/apache/gobblin/service/modules/orchestration/AzkabanClient.java:
##########
@@ -130,10 +137,12 @@ private void initializeClient() throws
AzkabanClientException {
private void initializeSessionManager() throws AzkabanClientException {
if (sessionManager == null) {
- this.sessionManager = new AzkabanSessionManager(this.httpClient,
- this.url,
- this.username,
- this.password);
+ try {
+ this.sessionManager = new AzkabanSessionManager(this.httpClient,
this.url, this.username, this.password);
+ }
+ catch(Exception e) {
+ log.error("Failed to initialize session manager due to: " + e);
Review Comment:
Same thing here, use the log.error throwable argument like I mentioned above.
https://logging.apache.org/log4j/2.x/log4j-api/apidocs/org/apache/logging/log4j/Logger.html#error-java.lang.CharSequence-java.lang.Throwable-
##########
gobblin-modules/gobblin-azkaban/src/main/java/org/apache/gobblin/service/modules/orchestration/AzkabanClient.java:
##########
@@ -117,7 +117,14 @@ protected AzkabanClient(String username,
.withWaitStrategy(WaitStrategies.exponentialWait(60, TimeUnit.SECONDS))
.withStopStrategy(StopStrategies.stopAfterAttempt(3))
.build();
- this.sessionId = this.sessionManager.fetchSession();
+ try {
+ this.sessionId = this.sessionManager.fetchSession();
+ }
+ catch (Exception e) {
+ this.sessionCreationTime = -1;
+ log.error("Failed to fetch session in constructor due to " + e);
Review Comment:
Can you rewrite this to, `log.error("Failed to fetch session during
initialization due to ", e)`
If you concat the exception with the log like this, it won't print out the
stack trace, and under the hood it just transforms the exception itself to the
object reference.
##########
gobblin-modules/gobblin-azkaban/src/main/java/org/apache/gobblin/service/modules/orchestration/AzkabanSpecProducer.java:
##########
@@ -84,6 +83,18 @@ public void close() throws IOException {
public Future<?> addSpec(Spec addedSpec) {
// If project already exists, execute it
try {
+ // If encountered Azkaban authentication issue on start up, attempt
authentication again.
+ // If still fails, only fail this specific flow
+ if (_sessionId == null) {
+ try {
+ attemptAzkabanAuthentication();
+ }
+ catch (RuntimeException e) {
Review Comment:
Usually we don't need to catch and rethrow the same type of exception, we
can try leaving this code as-is for now and letting the Azkaban client handle
this.
##########
gobblin-modules/gobblin-azkaban/src/main/java/org/apache/gobblin/service/modules/orchestration/AzkabanSpecProducer.java:
##########
@@ -47,14 +46,14 @@ public class AzkabanSpecProducer implements
SpecProducer<Spec>, Closeable {
public AzkabanSpecProducer(Config config, Optional<Logger> log) {
this._config = config;
try {
- // Initialize Azkaban client / producer and cache credentials
- String azkabanUsername =
_config.getString(ServiceAzkabanConfigKeys.AZKABAN_USERNAME_KEY);
- String azkabanPassword = getAzkabanPassword(_config);
- String azkabanServerUrl =
_config.getString(ServiceAzkabanConfigKeys.AZKABAN_SERVER_URL_KEY);
-
- _sessionId =
AzkabanAjaxAPIClient.authenticateAndGetSessionId(azkabanUsername,
azkabanPassword, azkabanServerUrl);
- } catch (IOException | EncoderException e) {
- throw new RuntimeException("Could not authenticate with Azkaban", e);
+ attemptAzkabanAuthentication();
+ }
+ catch (RuntimeException e) {
+ // Allow Azkaban authentication errors on start up, but just log the
error so GaaS can still start
+ _sessionId = null;
+ if (log.isPresent()) {
+ log.get().error("Could not authenticate with Azkaban due to:
{}".format(e.toString()));
Review Comment:
Same thing here
##########
gobblin-modules/gobblin-azkaban/src/main/java/org/apache/gobblin/service/modules/orchestration/AzkabanSpecProducer.java:
##########
@@ -84,6 +83,18 @@ public void close() throws IOException {
public Future<?> addSpec(Spec addedSpec) {
// If project already exists, execute it
try {
+ // If encountered Azkaban authentication issue on start up, attempt
authentication again.
Review Comment:
What happens if we didn't modify this code at all? I thought it should auto
attempt the authentication again because the sessionId is null, and the
sessionCreationTime is `-1` which will fulfill the renew requirements before
making a API request.
--
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]
