You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@gobblin.apache.org by GitBox <gi...@apache.org> on 2021/01/05 23:20:05 UTC

[GitHub] [incubator-gobblin] aplex opened a new pull request #3176: [GOBBLIN-1340] Extend Azkaban client

aplex opened a new pull request #3176:
URL: https://github.com/apache/incubator-gobblin/pull/3176


   We extend Azkaban client with API calls to check project existence,
   and get list of the flows. Log retrieval API is refactored to stream
   logs and use strongly-typed parameters.
   
   Azkaban client tests can now run concurrently, and should be more stable.
   
   These changes will be used in the end-to-end Gobblin testing framework.


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [incubator-gobblin] aplex commented on a change in pull request #3176: [GOBBLIN-1340] Extend Azkaban client

Posted by GitBox <gi...@apache.org>.
aplex commented on a change in pull request #3176:
URL: https://github.com/apache/incubator-gobblin/pull/3176#discussion_r551587674



##########
File path: gobblin-modules/gobblin-azkaban/src/main/java/org/apache/gobblin/service/modules/orchestration/AzkabanClient.java
##########
@@ -97,7 +86,8 @@ protected AzkabanClient(String username,
                           long sessionExpireInMin,
                           CloseableHttpClient httpClient,
                           SessionManager sessionManager,
-                          ExecutorService executorService)
+                          ExecutorService executorService,

Review comment:
       This is a builder constructor. I don't think it makes sense to have a backward-compatible overload every time we add a setting here, we'll end up wit a lot of overloads.

##########
File path: gobblin-modules/gobblin-azkaban/src/test/java/org/apache/gobblin/service/modules/orchestration/AzkabanClientTest.java
##########
@@ -48,9 +41,14 @@
  * Please check https://azkaban.github.io/azkaban/docs/latest/ for how to setup Azkaban-solo-server.
  */
 @Slf4j
+@Test(enabled = false)

Review comment:
       All those tests were actually disabled by default, because they require local Azkaban to be set up. We can look into automatically starting & shutting it down as a follow up improvement.




----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [incubator-gobblin] asfgit closed pull request #3176: [GOBBLIN-1340] Extend Azkaban client

Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #3176:
URL: https://github.com/apache/incubator-gobblin/pull/3176


   


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [incubator-gobblin] aplex commented on a change in pull request #3176: [GOBBLIN-1340] Extend Azkaban client

Posted by GitBox <gi...@apache.org>.
aplex commented on a change in pull request #3176:
URL: https://github.com/apache/incubator-gobblin/pull/3176#discussion_r551595499



##########
File path: gobblin-modules/gobblin-azkaban/src/main/java/org/apache/gobblin/service/modules/orchestration/AzkabanClient.java
##########
@@ -306,6 +317,22 @@ public AzkabanClientStatus deleteProject(String projectName) throws AzkabanClien
     return runWithRetry(callable, AzkabanClientStatus.class);
   }
 
+  /**
+   * Checks if the project with specified name exists in Azkaban
+   */
+  public Boolean projectExists(String projectName) throws AzkabanClientException {
+    try {
+      fetchProjectFlows(projectName);
+      return true;
+    } catch (AzkabanClientException e) {
+      // Azkaban does not return a strongly typed error code, so we are checking the message
+      if (e.getCause().getMessage().contains("doesn't exist")) {
+        return false;

Review comment:
       This is an outcome of a poorly-designed REST API on Azkaban side. It does not have a direct way to check for project existence, and it does not return machine-readable error codes - https://azkaban.readthedocs.io/en/latest/ajaxApi.html#fetch-jobs-of-a-flow . This can be unreliable, if they change the messages. I'll add a feature request for them to have better error reporting in the API.
   
   Since the error is an expected behavior here (when project does not exist), we shouldn't log it every time it happens - it will flood the logs when nothing bad has happened. 
   

##########
File path: gobblin-modules/gobblin-azkaban/src/main/java/org/apache/gobblin/service/modules/orchestration/AzkabanProjectFlowsStatus.java
##########
@@ -0,0 +1,42 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.gobblin.service.modules.orchestration;
+
+import lombok.AllArgsConstructor;
+import lombok.Getter;
+
+import java.util.List;
+
+public class AzkabanProjectFlowsStatus extends AzkabanClientStatus<AzkabanProjectFlowsStatus.Project> {

Review comment:
       Added a more extended comment in code. That's how all other API calls are designed for this Azkaban client.




----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [incubator-gobblin] aplex closed pull request #3176: [GOBBLIN-1340] Extend Azkaban client

Posted by GitBox <gi...@apache.org>.
aplex closed pull request #3176:
URL: https://github.com/apache/incubator-gobblin/pull/3176


   


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [incubator-gobblin] aplex commented on a change in pull request #3176: [GOBBLIN-1340] Extend Azkaban client

Posted by GitBox <gi...@apache.org>.
aplex commented on a change in pull request #3176:
URL: https://github.com/apache/incubator-gobblin/pull/3176#discussion_r551587799



##########
File path: gobblin-modules/gobblin-azkaban/src/test/java/org/apache/gobblin/service/modules/orchestration/AzkabanClientTest.java
##########
@@ -65,55 +63,73 @@ public void setup() throws Exception {
         .build();
   }
 
+  @BeforeMethod
+  public void testSetup() {
+    projectName = "test-project-" + System.currentTimeMillis() + "-" + UUID.randomUUID().toString().substring(0, 4);
+    description = "This is test project.";
+  }
+
+  @AfterMethod
+  public void testCleanup() throws AzkabanClientException {
+    this.client.deleteProject(projectName);
+  }
+
   @AfterClass
   public void cleanup() throws IOException {
     this.client.close();
   }
 
   private void ensureProjectExist(String projectName, String description) throws AzkabanClientException {
-    // make sure it is in a clean state
-    this.client.deleteProject(projectName);
-
-    // make sure the project is created successfully
     this.client.createProject(projectName, description);
   }
 
-  @Test(enabled = false)
-  public void testFetchLog() throws AzkabanClientException {
-    String execId = "11211956";
-    String jobId = "tracking-hourly-bucket1";
-
-    // fetch log
-    this.client.fetchExecutionLog(execId, jobId, "0", "100000000", new File("/tmp/sample.log"));
-  }
+  public void testFetchLog() throws Exception {
+    String flowName = "test-exec-flow";
+    String jobId = "test-exec-flow";
 
+    ensureProjectExist(projectName, description);
+    File zipFile = createAzkabanZip(flowName);
+    this.client.uploadProjectZip(projectName, zipFile);
 
-  @Test(enabled = false)
-  public void testCreateProject() throws AzkabanClientException {
-    String projectName = "project-create";
-    String description = "This is a create project test.";
+    AzkabanExecuteFlowStatus execStatus = this.client.executeFlow(projectName, flowName, Maps.newHashMap());
+    String execId = execStatus.getResponse().getExecId();
+
+    ByteArrayOutputStream logStream = null;
+    int maxTries = 10;

Review comment:
       There is a comment explaining this later on, moved it higher for clarity.




----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [incubator-gobblin] codecov-io commented on pull request #3176: [GOBBLIN-1340] Extend Azkaban client

Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #3176:
URL: https://github.com/apache/incubator-gobblin/pull/3176#issuecomment-754265912


   # [Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/3176?src=pr&el=h1) Report
   > Merging [#3176](https://codecov.io/gh/apache/incubator-gobblin/pull/3176?src=pr&el=desc) (c443a38) into [master](https://codecov.io/gh/apache/incubator-gobblin/commit/0aad6d42b68caffadd0981559131d5eb8acae814?el=desc) (0aad6d4) will **decrease** coverage by `37.08%`.
   > The diff coverage is `0.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-gobblin/pull/3176/graphs/tree.svg?width=650&height=150&src=pr&token=4MgURJ0bGc)](https://codecov.io/gh/apache/incubator-gobblin/pull/3176?src=pr&el=tree)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master   #3176       +/-   ##
   ============================================
   - Coverage     46.18%   9.09%   -37.09%     
   + Complexity     9719    1724     -7995     
   ============================================
     Files          2006    2015        +9     
     Lines         76854   77155      +301     
     Branches       8547    8559       +12     
   ============================================
   - Hits          35498    7021    -28477     
   - Misses        38058   69450    +31392     
   + Partials       3298     684     -2614     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-gobblin/pull/3176?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | [...pache/gobblin/configuration/ConfigurationKeys.java](https://codecov.io/gh/apache/incubator-gobblin/pull/3176/diff?src=pr&el=tree#diff-Z29iYmxpbi1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vY29uZmlndXJhdGlvbi9Db25maWd1cmF0aW9uS2V5cy5qYXZh) | `0.00% <ø> (ø)` | `0.00 <0.00> (ø)` | |
   | [...n/service/modules/orchestration/AzkabanClient.java](https://codecov.io/gh/apache/incubator-gobblin/pull/3176/diff?src=pr&el=tree#diff-Z29iYmxpbi1tb2R1bGVzL2dvYmJsaW4tYXprYWJhbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZ29iYmxpbi9zZXJ2aWNlL21vZHVsZXMvb3JjaGVzdHJhdGlvbi9BemthYmFuQ2xpZW50LmphdmE=) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (ø)` | |
   | [...e/modules/orchestration/AzkabanMultiCallables.java](https://codecov.io/gh/apache/incubator-gobblin/pull/3176/diff?src=pr&el=tree#diff-Z29iYmxpbi1tb2R1bGVzL2dvYmJsaW4tYXprYWJhbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZ29iYmxpbi9zZXJ2aWNlL21vZHVsZXMvb3JjaGVzdHJhdGlvbi9BemthYmFuTXVsdGlDYWxsYWJsZXMuamF2YQ==) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (ø)` | |
   | [...dules/orchestration/AzkabanProjectFlowsStatus.java](https://codecov.io/gh/apache/incubator-gobblin/pull/3176/diff?src=pr&el=tree#diff-Z29iYmxpbi1tb2R1bGVzL2dvYmJsaW4tYXprYWJhbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZ29iYmxpbi9zZXJ2aWNlL21vZHVsZXMvb3JjaGVzdHJhdGlvbi9BemthYmFuUHJvamVjdEZsb3dzU3RhdHVzLmphdmE=) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (?)` | |
   | [...n/service/modules/orchestration/SessionHelper.java](https://codecov.io/gh/apache/incubator-gobblin/pull/3176/diff?src=pr&el=tree#diff-Z29iYmxpbi1tb2R1bGVzL2dvYmJsaW4tYXprYWJhbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZ29iYmxpbi9zZXJ2aWNlL21vZHVsZXMvb3JjaGVzdHJhdGlvbi9TZXNzaW9uSGVscGVyLmphdmE=) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (ø)` | |
   | [...c/main/java/org/apache/gobblin/util/FileUtils.java](https://codecov.io/gh/apache/incubator-gobblin/pull/3176/diff?src=pr&el=tree#diff-Z29iYmxpbi11dGlsaXR5L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3V0aWwvRmlsZVV0aWxzLmphdmE=) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (-4.00%)` | |
   | [...n/java/org/apache/gobblin/fork/CopyableSchema.java](https://codecov.io/gh/apache/incubator-gobblin/pull/3176/diff?src=pr&el=tree#diff-Z29iYmxpbi1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2ZvcmsvQ29weWFibGVTY2hlbWEuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (-2.00%)` | |
   | [...java/org/apache/gobblin/stream/ControlMessage.java](https://codecov.io/gh/apache/incubator-gobblin/pull/3176/diff?src=pr&el=tree#diff-Z29iYmxpbi1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vc3RyZWFtL0NvbnRyb2xNZXNzYWdlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (-1.00%)` | |
   | [...va/org/apache/gobblin/dataset/DatasetResolver.java](https://codecov.io/gh/apache/incubator-gobblin/pull/3176/diff?src=pr&el=tree#diff-Z29iYmxpbi1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YXNldC9EYXRhc2V0UmVzb2x2ZXIuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (-3.00%)` | |
   | [...va/org/apache/gobblin/converter/EmptyIterable.java](https://codecov.io/gh/apache/incubator-gobblin/pull/3176/diff?src=pr&el=tree#diff-Z29iYmxpbi1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2NvbnZlcnRlci9FbXB0eUl0ZXJhYmxlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (-1.00%)` | |
   | ... and [1066 more](https://codecov.io/gh/apache/incubator-gobblin/pull/3176/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/3176?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/3176?src=pr&el=footer). Last update [0aad6d4...c443a38](https://codecov.io/gh/apache/incubator-gobblin/pull/3176?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [incubator-gobblin] codecov-io edited a comment on pull request #3176: [GOBBLIN-1340] Extend Azkaban client

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #3176:
URL: https://github.com/apache/incubator-gobblin/pull/3176#issuecomment-754265912


   # [Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/3176?src=pr&el=h1) Report
   > Merging [#3176](https://codecov.io/gh/apache/incubator-gobblin/pull/3176?src=pr&el=desc) (c443a38) into [master](https://codecov.io/gh/apache/incubator-gobblin/commit/0aad6d42b68caffadd0981559131d5eb8acae814?el=desc) (0aad6d4) will **decrease** coverage by `0.18%`.
   > The diff coverage is `0.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-gobblin/pull/3176/graphs/tree.svg?width=650&height=150&src=pr&token=4MgURJ0bGc)](https://codecov.io/gh/apache/incubator-gobblin/pull/3176?src=pr&el=tree)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #3176      +/-   ##
   ============================================
   - Coverage     46.18%   46.00%   -0.19%     
   - Complexity     9719     9751      +32     
   ============================================
     Files          2006     2015       +9     
     Lines         76854    77466     +612     
     Branches       8547     8606      +59     
   ============================================
   + Hits          35498    35635     +137     
   - Misses        38058    38523     +465     
   - Partials       3298     3308      +10     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-gobblin/pull/3176?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | [...pache/gobblin/configuration/ConfigurationKeys.java](https://codecov.io/gh/apache/incubator-gobblin/pull/3176/diff?src=pr&el=tree#diff-Z29iYmxpbi1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vY29uZmlndXJhdGlvbi9Db25maWd1cmF0aW9uS2V5cy5qYXZh) | `0.00% <ø> (ø)` | `0.00 <0.00> (ø)` | |
   | [...n/service/modules/orchestration/AzkabanClient.java](https://codecov.io/gh/apache/incubator-gobblin/pull/3176/diff?src=pr&el=tree#diff-Z29iYmxpbi1tb2R1bGVzL2dvYmJsaW4tYXprYWJhbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZ29iYmxpbi9zZXJ2aWNlL21vZHVsZXMvb3JjaGVzdHJhdGlvbi9BemthYmFuQ2xpZW50LmphdmE=) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (ø)` | |
   | [...e/modules/orchestration/AzkabanMultiCallables.java](https://codecov.io/gh/apache/incubator-gobblin/pull/3176/diff?src=pr&el=tree#diff-Z29iYmxpbi1tb2R1bGVzL2dvYmJsaW4tYXprYWJhbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZ29iYmxpbi9zZXJ2aWNlL21vZHVsZXMvb3JjaGVzdHJhdGlvbi9BemthYmFuTXVsdGlDYWxsYWJsZXMuamF2YQ==) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (ø)` | |
   | [...dules/orchestration/AzkabanProjectFlowsStatus.java](https://codecov.io/gh/apache/incubator-gobblin/pull/3176/diff?src=pr&el=tree#diff-Z29iYmxpbi1tb2R1bGVzL2dvYmJsaW4tYXprYWJhbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZ29iYmxpbi9zZXJ2aWNlL21vZHVsZXMvb3JjaGVzdHJhdGlvbi9BemthYmFuUHJvamVjdEZsb3dzU3RhdHVzLmphdmE=) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (?)` | |
   | [...n/service/modules/orchestration/SessionHelper.java](https://codecov.io/gh/apache/incubator-gobblin/pull/3176/diff?src=pr&el=tree#diff-Z29iYmxpbi1tb2R1bGVzL2dvYmJsaW4tYXprYWJhbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZ29iYmxpbi9zZXJ2aWNlL21vZHVsZXMvb3JjaGVzdHJhdGlvbi9TZXNzaW9uSGVscGVyLmphdmE=) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (ø)` | |
   | [...g/apache/gobblin/writer/PartitionedDataWriter.java](https://codecov.io/gh/apache/incubator-gobblin/pull/3176/diff?src=pr&el=tree#diff-Z29iYmxpbi1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3dyaXRlci9QYXJ0aXRpb25lZERhdGFXcml0ZXIuamF2YQ==) | `43.38% <0.00%> (-20.25%)` | `27.00% <0.00%> (-7.00%)` | |
   | [...he/gobblin/source/PartitionAwareFileRetriever.java](https://codecov.io/gh/apache/incubator-gobblin/pull/3176/diff?src=pr&el=tree#diff-Z29iYmxpbi1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NvdXJjZS9QYXJ0aXRpb25Bd2FyZUZpbGVSZXRyaWV2ZXIuamF2YQ==) | `48.14% <0.00%> (-7.41%)` | `0.00% <0.00%> (ø%)` | |
   | [...in/java/org/apache/gobblin/util/ClustersNames.java](https://codecov.io/gh/apache/incubator-gobblin/pull/3176/diff?src=pr&el=tree#diff-Z29iYmxpbi11dGlsaXR5L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3V0aWwvQ2x1c3RlcnNOYW1lcy5qYXZh) | `42.55% <0.00%> (-4.51%)` | `14.00% <0.00%> (+5.00%)` | :arrow_down: |
   | [...source/extractor/hadoop/HadoopFileInputSource.java](https://codecov.io/gh/apache/incubator-gobblin/pull/3176/diff?src=pr&el=tree#diff-Z29iYmxpbi1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NvdXJjZS9leHRyYWN0b3IvaGFkb29wL0hhZG9vcEZpbGVJbnB1dFNvdXJjZS5qYXZh) | `59.32% <0.00%> (-4.32%)` | `6.00% <0.00%> (ø%)` | |
   | [...lin/hive/metastore/HiveMetaStoreBasedRegister.java](https://codecov.io/gh/apache/incubator-gobblin/pull/3176/diff?src=pr&el=tree#diff-Z29iYmxpbi1oaXZlLXJlZ2lzdHJhdGlvbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZ29iYmxpbi9oaXZlL21ldGFzdG9yZS9IaXZlTWV0YVN0b3JlQmFzZWRSZWdpc3Rlci5qYXZh) | `7.15% <0.00%> (-3.13%)` | `2.00% <0.00%> (-3.00%)` | |
   | ... and [17 more](https://codecov.io/gh/apache/incubator-gobblin/pull/3176/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/3176?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/3176?src=pr&el=footer). Last update [0aad6d4...c443a38](https://codecov.io/gh/apache/incubator-gobblin/pull/3176?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [incubator-gobblin] autumnust commented on a change in pull request #3176: [GOBBLIN-1340] Extend Azkaban client

Posted by GitBox <gi...@apache.org>.
autumnust commented on a change in pull request #3176:
URL: https://github.com/apache/incubator-gobblin/pull/3176#discussion_r547453005



##########
File path: gobblin-modules/gobblin-azkaban/src/test/java/org/apache/gobblin/service/modules/orchestration/AzkabanClientTest.java
##########
@@ -65,55 +63,73 @@ public void setup() throws Exception {
         .build();
   }
 
+  @BeforeMethod
+  public void testSetup() {
+    projectName = "test-project-" + System.currentTimeMillis() + "-" + UUID.randomUUID().toString().substring(0, 4);
+    description = "This is test project.";
+  }
+
+  @AfterMethod
+  public void testCleanup() throws AzkabanClientException {
+    this.client.deleteProject(projectName);
+  }
+
   @AfterClass
   public void cleanup() throws IOException {
     this.client.close();
   }
 
   private void ensureProjectExist(String projectName, String description) throws AzkabanClientException {
-    // make sure it is in a clean state
-    this.client.deleteProject(projectName);
-
-    // make sure the project is created successfully
     this.client.createProject(projectName, description);
   }
 
-  @Test(enabled = false)
-  public void testFetchLog() throws AzkabanClientException {
-    String execId = "11211956";
-    String jobId = "tracking-hourly-bucket1";
-
-    // fetch log
-    this.client.fetchExecutionLog(execId, jobId, "0", "100000000", new File("/tmp/sample.log"));
-  }
+  public void testFetchLog() throws Exception {
+    String flowName = "test-exec-flow";
+    String jobId = "test-exec-flow";
 
+    ensureProjectExist(projectName, description);
+    File zipFile = createAzkabanZip(flowName);
+    this.client.uploadProjectZip(projectName, zipFile);
 
-  @Test(enabled = false)
-  public void testCreateProject() throws AzkabanClientException {
-    String projectName = "project-create";
-    String description = "This is a create project test.";
+    AzkabanExecuteFlowStatus execStatus = this.client.executeFlow(projectName, flowName, Maps.newHashMap());
+    String execId = execStatus.getResponse().getExecId();
+
+    ByteArrayOutputStream logStream = null;
+    int maxTries = 10;

Review comment:
       Considering smaller value?  

##########
File path: gobblin-modules/gobblin-azkaban/src/main/java/org/apache/gobblin/service/modules/orchestration/AzkabanClient.java
##########
@@ -97,7 +86,8 @@ protected AzkabanClient(String username,
                           long sessionExpireInMin,
                           CloseableHttpClient httpClient,
                           SessionManager sessionManager,
-                          ExecutorService executorService)
+                          ExecutorService executorService,

Review comment:
       Since it is a protected method, shall we maintain the backward compatibility ? 

##########
File path: gobblin-modules/gobblin-azkaban/src/main/java/org/apache/gobblin/service/modules/orchestration/AzkabanProjectFlowsStatus.java
##########
@@ -0,0 +1,42 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.gobblin.service.modules.orchestration;
+
+import lombok.AllArgsConstructor;
+import lombok.Getter;
+
+import java.util.List;
+
+public class AzkabanProjectFlowsStatus extends AzkabanClientStatus<AzkabanProjectFlowsStatus.Project> {

Review comment:
       I am not quite get the need for this class with two static inner class within it. Can you add a javadoc, and why this is needed ( Can the `Project` and `Flow` class themselves alone serve the same purpose? )

##########
File path: gobblin-modules/gobblin-azkaban/src/test/java/org/apache/gobblin/service/modules/orchestration/AzkabanClientTest.java
##########
@@ -48,9 +41,14 @@
  * Please check https://azkaban.github.io/azkaban/docs/latest/ for how to setup Azkaban-solo-server.
  */
 @Slf4j
+@Test(enabled = false)

Review comment:
       Intentional ? 

##########
File path: gobblin-modules/gobblin-azkaban/src/main/java/org/apache/gobblin/service/modules/orchestration/AzkabanClient.java
##########
@@ -306,6 +317,22 @@ public AzkabanClientStatus deleteProject(String projectName) throws AzkabanClien
     return runWithRetry(callable, AzkabanClientStatus.class);
   }
 
+  /**
+   * Checks if the project with specified name exists in Azkaban
+   */
+  public Boolean projectExists(String projectName) throws AzkabanClientException {
+    try {
+      fetchProjectFlows(projectName);
+      return true;
+    } catch (AzkabanClientException e) {
+      // Azkaban does not return a strongly typed error code, so we are checking the message
+      if (e.getCause().getMessage().contains("doesn't exist")) {
+        return false;

Review comment:
       Shall we print the error message anyway ? relying on a strict string matching here seems to be not reliable enough. 




----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [incubator-gobblin] codecov-io edited a comment on pull request #3176: [GOBBLIN-1340] Extend Azkaban client

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #3176:
URL: https://github.com/apache/incubator-gobblin/pull/3176#issuecomment-754265912


   # [Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/3176?src=pr&el=h1) Report
   > Merging [#3176](https://codecov.io/gh/apache/incubator-gobblin/pull/3176?src=pr&el=desc) (c443a38) into [master](https://codecov.io/gh/apache/incubator-gobblin/commit/0aad6d42b68caffadd0981559131d5eb8acae814?el=desc) (0aad6d4) will **decrease** coverage by `37.10%`.
   > The diff coverage is `0.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-gobblin/pull/3176/graphs/tree.svg?width=650&height=150&src=pr&token=4MgURJ0bGc)](https://codecov.io/gh/apache/incubator-gobblin/pull/3176?src=pr&el=tree)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master   #3176       +/-   ##
   ============================================
   - Coverage     46.18%   9.08%   -37.11%     
   + Complexity     9719    1727     -7992     
   ============================================
     Files          2006    2015        +9     
     Lines         76854   77466      +612     
     Branches       8547    8606       +59     
   ============================================
   - Hits          35498    7037    -28461     
   - Misses        38058   69743    +31685     
   + Partials       3298     686     -2612     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-gobblin/pull/3176?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | [...pache/gobblin/configuration/ConfigurationKeys.java](https://codecov.io/gh/apache/incubator-gobblin/pull/3176/diff?src=pr&el=tree#diff-Z29iYmxpbi1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vY29uZmlndXJhdGlvbi9Db25maWd1cmF0aW9uS2V5cy5qYXZh) | `0.00% <ø> (ø)` | `0.00 <0.00> (ø)` | |
   | [...n/service/modules/orchestration/AzkabanClient.java](https://codecov.io/gh/apache/incubator-gobblin/pull/3176/diff?src=pr&el=tree#diff-Z29iYmxpbi1tb2R1bGVzL2dvYmJsaW4tYXprYWJhbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZ29iYmxpbi9zZXJ2aWNlL21vZHVsZXMvb3JjaGVzdHJhdGlvbi9BemthYmFuQ2xpZW50LmphdmE=) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (ø)` | |
   | [...e/modules/orchestration/AzkabanMultiCallables.java](https://codecov.io/gh/apache/incubator-gobblin/pull/3176/diff?src=pr&el=tree#diff-Z29iYmxpbi1tb2R1bGVzL2dvYmJsaW4tYXprYWJhbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZ29iYmxpbi9zZXJ2aWNlL21vZHVsZXMvb3JjaGVzdHJhdGlvbi9BemthYmFuTXVsdGlDYWxsYWJsZXMuamF2YQ==) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (ø)` | |
   | [...dules/orchestration/AzkabanProjectFlowsStatus.java](https://codecov.io/gh/apache/incubator-gobblin/pull/3176/diff?src=pr&el=tree#diff-Z29iYmxpbi1tb2R1bGVzL2dvYmJsaW4tYXprYWJhbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZ29iYmxpbi9zZXJ2aWNlL21vZHVsZXMvb3JjaGVzdHJhdGlvbi9BemthYmFuUHJvamVjdEZsb3dzU3RhdHVzLmphdmE=) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (?)` | |
   | [...n/service/modules/orchestration/SessionHelper.java](https://codecov.io/gh/apache/incubator-gobblin/pull/3176/diff?src=pr&el=tree#diff-Z29iYmxpbi1tb2R1bGVzL2dvYmJsaW4tYXprYWJhbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZ29iYmxpbi9zZXJ2aWNlL21vZHVsZXMvb3JjaGVzdHJhdGlvbi9TZXNzaW9uSGVscGVyLmphdmE=) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (ø)` | |
   | [...c/main/java/org/apache/gobblin/util/FileUtils.java](https://codecov.io/gh/apache/incubator-gobblin/pull/3176/diff?src=pr&el=tree#diff-Z29iYmxpbi11dGlsaXR5L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3V0aWwvRmlsZVV0aWxzLmphdmE=) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (-4.00%)` | |
   | [...n/java/org/apache/gobblin/fork/CopyableSchema.java](https://codecov.io/gh/apache/incubator-gobblin/pull/3176/diff?src=pr&el=tree#diff-Z29iYmxpbi1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2ZvcmsvQ29weWFibGVTY2hlbWEuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (-2.00%)` | |
   | [...java/org/apache/gobblin/stream/ControlMessage.java](https://codecov.io/gh/apache/incubator-gobblin/pull/3176/diff?src=pr&el=tree#diff-Z29iYmxpbi1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vc3RyZWFtL0NvbnRyb2xNZXNzYWdlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (-1.00%)` | |
   | [...va/org/apache/gobblin/dataset/DatasetResolver.java](https://codecov.io/gh/apache/incubator-gobblin/pull/3176/diff?src=pr&el=tree#diff-Z29iYmxpbi1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YXNldC9EYXRhc2V0UmVzb2x2ZXIuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (-3.00%)` | |
   | [...va/org/apache/gobblin/converter/EmptyIterable.java](https://codecov.io/gh/apache/incubator-gobblin/pull/3176/diff?src=pr&el=tree#diff-Z29iYmxpbi1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2NvbnZlcnRlci9FbXB0eUl0ZXJhYmxlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (-1.00%)` | |
   | ... and [1066 more](https://codecov.io/gh/apache/incubator-gobblin/pull/3176/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/3176?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/3176?src=pr&el=footer). Last update [0aad6d4...c443a38](https://codecov.io/gh/apache/incubator-gobblin/pull/3176?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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:
users@infra.apache.org