[
https://issues.apache.org/jira/browse/GOBBLIN-1695?focusedWorklogId=806768&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-806768
]
ASF GitHub Bot logged work on GOBBLIN-1695:
-------------------------------------------
Author: ASF GitHub Bot
Created on: 07/Sep/22 17:46
Start Date: 07/Sep/22 17:46
Worklog Time Spent: 10m
Work Description: 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.
Issue Time Tracking
-------------------
Worklog Id: (was: 806768)
Time Spent: 0.5h (was: 20m)
> GaaS shouldn't block deployment on adding spec executors
> --------------------------------------------------------
>
> Key: GOBBLIN-1695
> URL: https://issues.apache.org/jira/browse/GOBBLIN-1695
> Project: Apache Gobblin
> Issue Type: Bug
> Reporter: Andy Jiang
> Priority: Major
> Time Spent: 0.5h
> Remaining Estimate: 0h
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
