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 2020/01/09 18:52:08 UTC

[GitHub] [incubator-gobblin] jack-moseley opened a new pull request #2862: [GOBBLIN-1017] Deprecate FlowStatus in favor of FlowExecution and add endpoint to kill flows

jack-moseley opened a new pull request #2862: [GOBBLIN-1017] Deprecate FlowStatus in favor of FlowExecution and add endpoint to kill flows
URL: https://github.com/apache/incubator-gobblin/pull/2862
 
 
   Dear Gobblin maintainers,
   
   Please accept this PR. I understand that it will not be reviewed until I have checked off all the steps below!
   
   
   ### JIRA
   - [x] My PR addresses the following [Gobblin JIRA](https://issues.apache.org/jira/browse/GOBBLIN/) issues and references them in the PR title. For example, "[GOBBLIN-XXX] My Gobblin PR"
       - https://issues.apache.org/jira/browse/GOBBLIN-1017
   
   
   ### Description
   - [x] Here are some details about my PR, including screenshots (if applicable):
   
   - Deprecate `FlowStatus` in favor of `FlowExecution` (since it makes more sense to call delete/update etc. on a flow execution instead of a flow status)
   - Create `FlowExecutionResource` and `FlowExecutionClient` with most code shared from the original flow status (get calls still work exactly the same)
   - Add a delete endpoint for flow executions which sends a message to the `DagManager` telling it to kill the corresponding flow
   
   ### Tests
   - [x] My PR adds the following unit tests __OR__ does not need testing for this extremely good reason:
   
   
   ### Commits
   - [x] My commits all reference JIRA issues in their subject lines, and I have squashed multiple commits if they address the same issue. In addition, my commits follow the guidelines from "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)":
       1. Subject is separated from body by a blank line
       2. Subject is limited to 50 characters
       3. Subject does not end with a period
       4. Subject uses the imperative mood ("add", not "adding")
       5. Body wraps at 72 characters
       6. Body explains "what" and "why", not "how"
   
   

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [incubator-gobblin] asfgit closed pull request #2862: [GOBBLIN-1017] Deprecate FlowStatus in favor of FlowExecution and add endpoint to kill flows

Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #2862: [GOBBLIN-1017] Deprecate FlowStatus in favor of FlowExecution and add endpoint to kill flows
URL: https://github.com/apache/incubator-gobblin/pull/2862
 
 
   

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [incubator-gobblin] sv2000 commented on a change in pull request #2862: [GOBBLIN-1017] Deprecate FlowStatus in favor of FlowExecution and add endpoint to kill flows

Posted by GitBox <gi...@apache.org>.
sv2000 commented on a change in pull request #2862: [GOBBLIN-1017] Deprecate FlowStatus in favor of FlowExecution and add endpoint to kill flows
URL: https://github.com/apache/incubator-gobblin/pull/2862#discussion_r371853294
 
 

 ##########
 File path: gobblin-restli/gobblin-flow-config-service/gobblin-flow-config-service-client/src/main/java/org/apache/gobblin/service/FlowExecutionClient.java
 ##########
 @@ -0,0 +1,170 @@
+/*
+ * 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;
+
+import java.io.Closeable;
+import java.io.IOException;
+import java.util.Collections;
+import java.util.List;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.common.base.Optional;
+import com.google.common.base.Preconditions;
+import com.linkedin.common.callback.FutureCallback;
+import com.linkedin.common.util.None;
+import com.linkedin.r2.RemoteInvocationException;
+import com.linkedin.r2.transport.common.Client;
+import com.linkedin.r2.transport.common.bridge.client.TransportClientAdapter;
+import com.linkedin.r2.transport.http.client.HttpClientFactory;
+import com.linkedin.restli.client.FindRequest;
+import com.linkedin.restli.client.GetRequest;
+import com.linkedin.restli.client.Response;
+import com.linkedin.restli.client.RestClient;
+import com.linkedin.restli.common.CollectionResponse;
+import com.linkedin.restli.common.ComplexResourceKey;
+import com.linkedin.restli.common.EmptyRecord;
+
+
+/**
+ * Flow execution client for REST flow execution server
+ */
+public class FlowExecutionClient implements Closeable {
+  private static final Logger LOG = LoggerFactory.getLogger(FlowExecutionClient.class);
+
+  private Optional<HttpClientFactory> _httpClientFactory;
+  private Optional<RestClient> _restClient;
+  private final FlowexecutionsRequestBuilders _flowexecutionsRequestBuilders;
+
+  /**
+   * Construct a {@link FlowExecutionClient} to communicate with http flow execution server at URI serverUri
+   * @param serverUri address and port of the REST server
+   */
+  public FlowExecutionClient(String serverUri) {
+    LOG.debug("FlowConfigClient with serverUri " + serverUri);
+
+    _httpClientFactory = Optional.of(new HttpClientFactory());
+    Client r2Client = new TransportClientAdapter(_httpClientFactory.get().getClient(Collections.<String, String>emptyMap()));
+    _restClient = Optional.of(new RestClient(r2Client, serverUri));
+
+    _flowexecutionsRequestBuilders = createRequestBuilders();
+  }
+
+  /**
+   * Construct a {@link FlowExecutionClient} to communicate with http flow execution server at URI serverUri
+   * @param restClient restClient to send restli request
+   */
+  public FlowExecutionClient(RestClient restClient) {
+    LOG.debug("FlowConfigClient with restClient " + restClient);
+
+    _httpClientFactory = Optional.absent();
+    _restClient = Optional.of(restClient);
+
+    _flowexecutionsRequestBuilders = createRequestBuilders();
+  }
+
+  protected FlowexecutionsRequestBuilders createRequestBuilders() {
+    return new FlowexecutionsRequestBuilders();
+  }
+
+  /**
+   * Get a flow execution
+   * @param flowStatusId identifier of flow execution to get
+   * @return a {@link FlowExecution} with the flow execution
+   * @throws RemoteInvocationException
+   */
+  public FlowExecution getFlowExecution(FlowStatusId flowStatusId)
+      throws RemoteInvocationException {
+    LOG.debug("getFlowConfig with groupName " + flowStatusId.getFlowGroup() + " flowName " +
+        flowStatusId.getFlowName());
+
+    GetRequest<FlowExecution> getRequest = _flowexecutionsRequestBuilders.get()
+        .id(new ComplexResourceKey<>(flowStatusId, new EmptyRecord())).build();
+
+    Response<FlowExecution> response =
+        _restClient.get().sendRequest(getRequest).getResponse();
+    return response.getEntity();
+  }
+
+  /**
+   * Get the latest flow execution
+   * @param flowId identifier of flow execution to get
+   * @return a {@link FlowExecution}
+   * @throws RemoteInvocationException
+   */
+  public FlowExecution getLatestFlowExecution(FlowId flowId)
+      throws RemoteInvocationException {
+    LOG.debug("getFlowConfig with groupName " + flowId.getFlowGroup() + " flowName " +
+        flowId.getFlowName());
+
+    FindRequest<FlowExecution> findRequest = _flowexecutionsRequestBuilders.findByLatestFlowExecution().flowIdParam(flowId).build();
+
+    Response<CollectionResponse<FlowExecution>> response =
+        _restClient.get().sendRequest(findRequest).getResponse();
+
+    List<FlowExecution> flowExecutionList = response.getEntity().getElements();
+
+    if (flowExecutionList.isEmpty()) {
+      return null;
+    } else {
+      Preconditions.checkArgument(flowExecutionList.size() == 1);
+      return flowExecutionList.get(0);
+    }
+  }
+
+  /**
+   * Get the latest flow execution
 
 Review comment:
   Get the latest "k" flow executions?

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [incubator-gobblin] jack-moseley commented on a change in pull request #2862: [GOBBLIN-1017] Deprecate FlowStatus in favor of FlowExecution and add endpoint to kill flows

Posted by GitBox <gi...@apache.org>.
jack-moseley commented on a change in pull request #2862: [GOBBLIN-1017] Deprecate FlowStatus in favor of FlowExecution and add endpoint to kill flows
URL: https://github.com/apache/incubator-gobblin/pull/2862#discussion_r372133067
 
 

 ##########
 File path: gobblin-restli/gobblin-flow-config-service/gobblin-flow-config-service-client/src/main/java/org/apache/gobblin/service/FlowExecutionClient.java
 ##########
 @@ -0,0 +1,170 @@
+/*
+ * 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;
+
+import java.io.Closeable;
+import java.io.IOException;
+import java.util.Collections;
+import java.util.List;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.common.base.Optional;
+import com.google.common.base.Preconditions;
+import com.linkedin.common.callback.FutureCallback;
+import com.linkedin.common.util.None;
+import com.linkedin.r2.RemoteInvocationException;
+import com.linkedin.r2.transport.common.Client;
+import com.linkedin.r2.transport.common.bridge.client.TransportClientAdapter;
+import com.linkedin.r2.transport.http.client.HttpClientFactory;
+import com.linkedin.restli.client.FindRequest;
+import com.linkedin.restli.client.GetRequest;
+import com.linkedin.restli.client.Response;
+import com.linkedin.restli.client.RestClient;
+import com.linkedin.restli.common.CollectionResponse;
+import com.linkedin.restli.common.ComplexResourceKey;
+import com.linkedin.restli.common.EmptyRecord;
+
+
+/**
+ * Flow execution client for REST flow execution server
+ */
+public class FlowExecutionClient implements Closeable {
+  private static final Logger LOG = LoggerFactory.getLogger(FlowExecutionClient.class);
+
+  private Optional<HttpClientFactory> _httpClientFactory;
+  private Optional<RestClient> _restClient;
+  private final FlowexecutionsRequestBuilders _flowexecutionsRequestBuilders;
+
+  /**
+   * Construct a {@link FlowExecutionClient} to communicate with http flow execution server at URI serverUri
+   * @param serverUri address and port of the REST server
+   */
+  public FlowExecutionClient(String serverUri) {
+    LOG.debug("FlowConfigClient with serverUri " + serverUri);
+
+    _httpClientFactory = Optional.of(new HttpClientFactory());
+    Client r2Client = new TransportClientAdapter(_httpClientFactory.get().getClient(Collections.<String, String>emptyMap()));
+    _restClient = Optional.of(new RestClient(r2Client, serverUri));
+
+    _flowexecutionsRequestBuilders = createRequestBuilders();
+  }
+
+  /**
+   * Construct a {@link FlowExecutionClient} to communicate with http flow execution server at URI serverUri
+   * @param restClient restClient to send restli request
+   */
+  public FlowExecutionClient(RestClient restClient) {
+    LOG.debug("FlowConfigClient with restClient " + restClient);
+
+    _httpClientFactory = Optional.absent();
+    _restClient = Optional.of(restClient);
+
+    _flowexecutionsRequestBuilders = createRequestBuilders();
+  }
+
+  protected FlowexecutionsRequestBuilders createRequestBuilders() {
+    return new FlowexecutionsRequestBuilders();
+  }
+
+  /**
+   * Get a flow execution
+   * @param flowStatusId identifier of flow execution to get
+   * @return a {@link FlowExecution} with the flow execution
+   * @throws RemoteInvocationException
+   */
+  public FlowExecution getFlowExecution(FlowStatusId flowStatusId)
+      throws RemoteInvocationException {
+    LOG.debug("getFlowConfig with groupName " + flowStatusId.getFlowGroup() + " flowName " +
+        flowStatusId.getFlowName());
+
+    GetRequest<FlowExecution> getRequest = _flowexecutionsRequestBuilders.get()
+        .id(new ComplexResourceKey<>(flowStatusId, new EmptyRecord())).build();
+
+    Response<FlowExecution> response =
+        _restClient.get().sendRequest(getRequest).getResponse();
+    return response.getEntity();
+  }
+
+  /**
+   * Get the latest flow execution
+   * @param flowId identifier of flow execution to get
+   * @return a {@link FlowExecution}
+   * @throws RemoteInvocationException
+   */
+  public FlowExecution getLatestFlowExecution(FlowId flowId)
+      throws RemoteInvocationException {
+    LOG.debug("getFlowConfig with groupName " + flowId.getFlowGroup() + " flowName " +
+        flowId.getFlowName());
+
+    FindRequest<FlowExecution> findRequest = _flowexecutionsRequestBuilders.findByLatestFlowExecution().flowIdParam(flowId).build();
+
+    Response<CollectionResponse<FlowExecution>> response =
+        _restClient.get().sendRequest(findRequest).getResponse();
+
+    List<FlowExecution> flowExecutionList = response.getEntity().getElements();
+
+    if (flowExecutionList.isEmpty()) {
+      return null;
+    } else {
+      Preconditions.checkArgument(flowExecutionList.size() == 1);
+      return flowExecutionList.get(0);
+    }
+  }
+
+  /**
+   * Get the latest flow execution
 
 Review comment:
   Fixed and also updated some incorrect log messages copied from FlowStatusClient

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [incubator-gobblin] jack-moseley commented on a change in pull request #2862: [GOBBLIN-1017] Deprecate FlowStatus in favor of FlowExecution and add endpoint to kill flows

Posted by GitBox <gi...@apache.org>.
jack-moseley commented on a change in pull request #2862: [GOBBLIN-1017] Deprecate FlowStatus in favor of FlowExecution and add endpoint to kill flows
URL: https://github.com/apache/incubator-gobblin/pull/2862#discussion_r372133869
 
 

 ##########
 File path: gobblin-restli/gobblin-flow-config-service/gobblin-flow-config-service-client/src/main/java/org/apache/gobblin/service/FlowExecutionClient.java
 ##########
 @@ -0,0 +1,170 @@
+/*
+ * 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;
+
+import java.io.Closeable;
+import java.io.IOException;
+import java.util.Collections;
+import java.util.List;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.common.base.Optional;
+import com.google.common.base.Preconditions;
+import com.linkedin.common.callback.FutureCallback;
+import com.linkedin.common.util.None;
+import com.linkedin.r2.RemoteInvocationException;
+import com.linkedin.r2.transport.common.Client;
+import com.linkedin.r2.transport.common.bridge.client.TransportClientAdapter;
+import com.linkedin.r2.transport.http.client.HttpClientFactory;
+import com.linkedin.restli.client.FindRequest;
+import com.linkedin.restli.client.GetRequest;
+import com.linkedin.restli.client.Response;
+import com.linkedin.restli.client.RestClient;
+import com.linkedin.restli.common.CollectionResponse;
+import com.linkedin.restli.common.ComplexResourceKey;
+import com.linkedin.restli.common.EmptyRecord;
+
+
+/**
+ * Flow execution client for REST flow execution server
+ */
+public class FlowExecutionClient implements Closeable {
+  private static final Logger LOG = LoggerFactory.getLogger(FlowExecutionClient.class);
+
+  private Optional<HttpClientFactory> _httpClientFactory;
+  private Optional<RestClient> _restClient;
+  private final FlowexecutionsRequestBuilders _flowexecutionsRequestBuilders;
+
+  /**
+   * Construct a {@link FlowExecutionClient} to communicate with http flow execution server at URI serverUri
+   * @param serverUri address and port of the REST server
+   */
+  public FlowExecutionClient(String serverUri) {
+    LOG.debug("FlowConfigClient with serverUri " + serverUri);
+
+    _httpClientFactory = Optional.of(new HttpClientFactory());
+    Client r2Client = new TransportClientAdapter(_httpClientFactory.get().getClient(Collections.<String, String>emptyMap()));
+    _restClient = Optional.of(new RestClient(r2Client, serverUri));
+
+    _flowexecutionsRequestBuilders = createRequestBuilders();
+  }
+
+  /**
+   * Construct a {@link FlowExecutionClient} to communicate with http flow execution server at URI serverUri
+   * @param restClient restClient to send restli request
+   */
+  public FlowExecutionClient(RestClient restClient) {
+    LOG.debug("FlowConfigClient with restClient " + restClient);
+
+    _httpClientFactory = Optional.absent();
+    _restClient = Optional.of(restClient);
+
+    _flowexecutionsRequestBuilders = createRequestBuilders();
+  }
+
+  protected FlowexecutionsRequestBuilders createRequestBuilders() {
+    return new FlowexecutionsRequestBuilders();
+  }
+
+  /**
+   * Get a flow execution
+   * @param flowStatusId identifier of flow execution to get
+   * @return a {@link FlowExecution} with the flow execution
+   * @throws RemoteInvocationException
+   */
+  public FlowExecution getFlowExecution(FlowStatusId flowStatusId)
+      throws RemoteInvocationException {
+    LOG.debug("getFlowConfig with groupName " + flowStatusId.getFlowGroup() + " flowName " +
+        flowStatusId.getFlowName());
+
+    GetRequest<FlowExecution> getRequest = _flowexecutionsRequestBuilders.get()
+        .id(new ComplexResourceKey<>(flowStatusId, new EmptyRecord())).build();
+
+    Response<FlowExecution> response =
+        _restClient.get().sendRequest(getRequest).getResponse();
+    return response.getEntity();
+  }
+
+  /**
+   * Get the latest flow execution
+   * @param flowId identifier of flow execution to get
+   * @return a {@link FlowExecution}
+   * @throws RemoteInvocationException
+   */
+  public FlowExecution getLatestFlowExecution(FlowId flowId)
+      throws RemoteInvocationException {
+    LOG.debug("getFlowConfig with groupName " + flowId.getFlowGroup() + " flowName " +
+        flowId.getFlowName());
+
+    FindRequest<FlowExecution> findRequest = _flowexecutionsRequestBuilders.findByLatestFlowExecution().flowIdParam(flowId).build();
+
+    Response<CollectionResponse<FlowExecution>> response =
+        _restClient.get().sendRequest(findRequest).getResponse();
+
+    List<FlowExecution> flowExecutionList = response.getEntity().getElements();
 
 Review comment:
   I don't think it's necessary since if there's no flow executions, it already will throw a wrapped rest exception when sending the request in the line before.

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [incubator-gobblin] codecov-io edited a comment on issue #2862: [GOBBLIN-1017] Deprecate FlowStatus in favor of FlowExecution and add endpoint to kill flows

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #2862: [GOBBLIN-1017] Deprecate FlowStatus in favor of FlowExecution and add endpoint to kill flows
URL: https://github.com/apache/incubator-gobblin/pull/2862#issuecomment-572733455
 
 
   # [Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2862?src=pr&el=h1) Report
   > Merging [#2862](https://codecov.io/gh/apache/incubator-gobblin/pull/2862?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-gobblin/commit/cd53dcfa2c046f53db29689e1d687bf4412565aa?src=pr&el=desc) will **decrease** coverage by `0.06%`.
   > The diff coverage is `7.48%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-gobblin/pull/2862/graphs/tree.svg?width=650&token=4MgURJ0bGc&height=150&src=pr)](https://codecov.io/gh/apache/incubator-gobblin/pull/2862?src=pr&el=tree)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #2862      +/-   ##
   ============================================
   - Coverage      45.8%   45.74%   -0.07%     
   + Complexity     9111     9110       -1     
   ============================================
     Files          1915     1918       +3     
     Lines         72285    72374      +89     
     Branches       7972     7979       +7     
   ============================================
   - Hits          33109    33105       -4     
   - Misses        36150    36241      +91     
   - Partials       3026     3028       +2
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-gobblin/pull/2862?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | [...ache/gobblin/service/monitoring/KillFlowEvent.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2862/diff?src=pr&el=tree#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9uaXRvcmluZy9LaWxsRmxvd0V2ZW50LmphdmE=) | `0% <0%> (ø)` | `0 <0> (?)` | |
   | [...obblin/service/monitoring/FlowStatusGenerator.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2862/diff?src=pr&el=tree#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9uaXRvcmluZy9GbG93U3RhdHVzR2VuZXJhdG9yLmphdmE=) | `69.69% <0%> (-4.5%)` | `11 <0> (ø)` | |
   | [...org/apache/gobblin/service/FlowStatusResource.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2862/diff?src=pr&el=tree#diff-Z29iYmxpbi1yZXN0bGkvZ29iYmxpbi1mbG93LWNvbmZpZy1zZXJ2aWNlL2dvYmJsaW4tZmxvdy1jb25maWctc2VydmljZS1zZXJ2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vc2VydmljZS9GbG93U3RhdHVzUmVzb3VyY2UuamF2YQ==) | `0% <0%> (ø)` | `0 <0> (ø)` | :arrow_down: |
   | [...rg/apache/gobblin/service/FlowExecutionClient.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2862/diff?src=pr&el=tree#diff-Z29iYmxpbi1yZXN0bGkvZ29iYmxpbi1mbG93LWNvbmZpZy1zZXJ2aWNlL2dvYmJsaW4tZmxvdy1jb25maWctc2VydmljZS1jbGllbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vc2VydmljZS9GbG93RXhlY3V0aW9uQ2xpZW50LmphdmE=) | `0% <0%> (ø)` | `0 <0> (?)` | |
   | [.../apache/gobblin/service/FlowExecutionResource.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2862/diff?src=pr&el=tree#diff-Z29iYmxpbi1yZXN0bGkvZ29iYmxpbi1mbG93LWNvbmZpZy1zZXJ2aWNlL2dvYmJsaW4tZmxvdy1jb25maWctc2VydmljZS1zZXJ2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vc2VydmljZS9GbG93RXhlY3V0aW9uUmVzb3VyY2UuamF2YQ==) | `0% <0%> (ø)` | `0 <0> (?)` | |
   | [...blin/service/modules/orchestration/DagManager.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2862/diff?src=pr&el=tree#diff-Z29iYmxpbi1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9kdWxlcy9vcmNoZXN0cmF0aW9uL0RhZ01hbmFnZXIuamF2YQ==) | `76.54% <50%> (-1.04%)` | `13 <1> (+1)` | |
   | [...in/service/modules/core/GobblinServiceManager.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2862/diff?src=pr&el=tree#diff-Z29iYmxpbi1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9kdWxlcy9jb3JlL0dvYmJsaW5TZXJ2aWNlTWFuYWdlci5qYXZh) | `54.71% <50%> (-0.04%)` | `25 <0> (ø)` | |
   | [...a/org/apache/gobblin/service/FlowStatusClient.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2862/diff?src=pr&el=tree#diff-Z29iYmxpbi1yZXN0bGkvZ29iYmxpbi1mbG93LWNvbmZpZy1zZXJ2aWNlL2dvYmJsaW4tZmxvdy1jb25maWctc2VydmljZS1jbGllbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vc2VydmljZS9GbG93U3RhdHVzQ2xpZW50LmphdmE=) | `72.34% <80%> (ø)` | `7 <3> (ø)` | :arrow_down: |
   | [...e/job\_catalog/FSPathAlterationListenerAdaptor.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2862/diff?src=pr&el=tree#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3J1bnRpbWUvam9iX2NhdGFsb2cvRlNQYXRoQWx0ZXJhdGlvbkxpc3RlbmVyQWRhcHRvci5qYXZh) | `62.5% <0%> (-20.84%)` | `3% <0%> (-1%)` | |
   | [...in/java/org/apache/gobblin/cluster/HelixUtils.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2862/diff?src=pr&el=tree#diff-Z29iYmxpbi1jbHVzdGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2NsdXN0ZXIvSGVsaXhVdGlscy5qYXZh) | `35.51% <0%> (-3.74%)` | `12% <0%> (-1%)` | |
   | ... and [11 more](https://codecov.io/gh/apache/incubator-gobblin/pull/2862/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2862?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/2862?src=pr&el=footer). Last update [cd53dcf...2402400](https://codecov.io/gh/apache/incubator-gobblin/pull/2862?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


With regards,
Apache Git Services

[GitHub] [incubator-gobblin] codecov-io edited a comment on issue #2862: [GOBBLIN-1017] Deprecate FlowStatus in favor of FlowExecution and add endpoint to kill flows

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #2862: [GOBBLIN-1017] Deprecate FlowStatus in favor of FlowExecution and add endpoint to kill flows
URL: https://github.com/apache/incubator-gobblin/pull/2862#issuecomment-572733455
 
 
   # [Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2862?src=pr&el=h1) Report
   > Merging [#2862](https://codecov.io/gh/apache/incubator-gobblin/pull/2862?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-gobblin/commit/d18783df7ab29b161cca59ca907fb47e1366b358?src=pr&el=desc) will **increase** coverage by `0.95%`.
   > The diff coverage is `7.04%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-gobblin/pull/2862/graphs/tree.svg?width=650&token=4MgURJ0bGc&height=150&src=pr)](https://codecov.io/gh/apache/incubator-gobblin/pull/2862?src=pr&el=tree)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #2862      +/-   ##
   ============================================
   + Coverage      44.8%   45.75%   +0.95%     
   - Complexity     8933     9109     +176     
   ============================================
     Files          1917     1918       +1     
     Lines         72131    72366     +235     
     Branches       7956     7977      +21     
   ============================================
   + Hits          32316    33109     +793     
   + Misses        36832    36231     -601     
   - Partials       2983     3026      +43
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-gobblin/pull/2862?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | [...a/org/apache/gobblin/service/FlowStatusClient.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2862/diff?src=pr&el=tree#diff-Z29iYmxpbi1yZXN0bGkvZ29iYmxpbi1mbG93LWNvbmZpZy1zZXJ2aWNlL2dvYmJsaW4tZmxvdy1jb25maWctc2VydmljZS1jbGllbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vc2VydmljZS9GbG93U3RhdHVzQ2xpZW50LmphdmE=) | `72.34% <ø> (ø)` | `7 <0> (ø)` | :arrow_down: |
   | [...ache/gobblin/service/monitoring/KillFlowEvent.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2862/diff?src=pr&el=tree#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9uaXRvcmluZy9LaWxsRmxvd0V2ZW50LmphdmE=) | `0% <0%> (ø)` | `0 <0> (?)` | |
   | [...obblin/service/monitoring/FlowStatusGenerator.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2862/diff?src=pr&el=tree#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9uaXRvcmluZy9GbG93U3RhdHVzR2VuZXJhdG9yLmphdmE=) | `69.69% <0%> (-4.5%)` | `11 <0> (ø)` | |
   | [...org/apache/gobblin/service/FlowStatusResource.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2862/diff?src=pr&el=tree#diff-Z29iYmxpbi1yZXN0bGkvZ29iYmxpbi1mbG93LWNvbmZpZy1zZXJ2aWNlL2dvYmJsaW4tZmxvdy1jb25maWctc2VydmljZS1zZXJ2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vc2VydmljZS9GbG93U3RhdHVzUmVzb3VyY2UuamF2YQ==) | `0% <0%> (ø)` | `0 <0> (ø)` | :arrow_down: |
   | [...rg/apache/gobblin/service/FlowExecutionClient.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2862/diff?src=pr&el=tree#diff-Z29iYmxpbi1yZXN0bGkvZ29iYmxpbi1mbG93LWNvbmZpZy1zZXJ2aWNlL2dvYmJsaW4tZmxvdy1jb25maWctc2VydmljZS1jbGllbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vc2VydmljZS9GbG93RXhlY3V0aW9uQ2xpZW50LmphdmE=) | `0% <0%> (ø)` | `0 <0> (?)` | |
   | [.../apache/gobblin/service/FlowExecutionResource.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2862/diff?src=pr&el=tree#diff-Z29iYmxpbi1yZXN0bGkvZ29iYmxpbi1mbG93LWNvbmZpZy1zZXJ2aWNlL2dvYmJsaW4tZmxvdy1jb25maWctc2VydmljZS1zZXJ2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vc2VydmljZS9GbG93RXhlY3V0aW9uUmVzb3VyY2UuamF2YQ==) | `0% <0%> (ø)` | `0 <0> (?)` | |
   | [...in/service/modules/core/GobblinServiceManager.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2862/diff?src=pr&el=tree#diff-Z29iYmxpbi1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9kdWxlcy9jb3JlL0dvYmJsaW5TZXJ2aWNlTWFuYWdlci5qYXZh) | `54.71% <50%> (-0.04%)` | `24 <0> (-1)` | |
   | [...blin/service/modules/orchestration/DagManager.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2862/diff?src=pr&el=tree#diff-Z29iYmxpbi1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9kdWxlcy9vcmNoZXN0cmF0aW9uL0RhZ01hbmFnZXIuamF2YQ==) | `76.54% <75%> (-2.83%)` | `13 <1> (+1)` | |
   | [.../gobblin/cluster/GobblinHelixMessagingService.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2862/diff?src=pr&el=tree#diff-Z29iYmxpbi1jbHVzdGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2NsdXN0ZXIvR29iYmxpbkhlbGl4TWVzc2FnaW5nU2VydmljZS5qYXZh) | `63.04% <0%> (-2.13%)` | `4% <0%> (ø)` | |
   | ... and [65 more](https://codecov.io/gh/apache/incubator-gobblin/pull/2862/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2862?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/2862?src=pr&el=footer). Last update [d18783d...07a50bd](https://codecov.io/gh/apache/incubator-gobblin/pull/2862?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


With regards,
Apache Git Services

[GitHub] [incubator-gobblin] codecov-io commented on issue #2862: [GOBBLIN-1017] Deprecate FlowStatus in favor of FlowExecution and add endpoint to kill flows

Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #2862: [GOBBLIN-1017] Deprecate FlowStatus in favor of FlowExecution and add endpoint to kill flows
URL: https://github.com/apache/incubator-gobblin/pull/2862#issuecomment-572733455
 
 
   # [Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2862?src=pr&el=h1) Report
   > Merging [#2862](https://codecov.io/gh/apache/incubator-gobblin/pull/2862?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-gobblin/commit/d18783df7ab29b161cca59ca907fb47e1366b358?src=pr&el=desc) will **increase** coverage by `0.92%`.
   > The diff coverage is `6.19%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-gobblin/pull/2862/graphs/tree.svg?width=650&token=4MgURJ0bGc&height=150&src=pr)](https://codecov.io/gh/apache/incubator-gobblin/pull/2862?src=pr&el=tree)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #2862      +/-   ##
   ============================================
   + Coverage      44.8%   45.72%   +0.92%     
   - Complexity     8933     9103     +170     
   ============================================
     Files          1917     1920       +3     
     Lines         72131    72183      +52     
     Branches       7956     7957       +1     
   ============================================
   + Hits          32316    33005     +689     
   + Misses        36832    36153     -679     
   - Partials       2983     3025      +42
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-gobblin/pull/2862?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | [...a/org/apache/gobblin/service/FlowStatusClient.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2862/diff?src=pr&el=tree#diff-Z29iYmxpbi1yZXN0bGkvZ29iYmxpbi1mbG93LWNvbmZpZy1zZXJ2aWNlL2dvYmJsaW4tZmxvdy1jb25maWctc2VydmljZS1jbGllbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vc2VydmljZS9GbG93U3RhdHVzQ2xpZW50LmphdmE=) | `72.34% <ø> (ø)` | `7 <0> (ø)` | :arrow_down: |
   | [...ache/gobblin/service/monitoring/KillFlowEvent.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2862/diff?src=pr&el=tree#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9uaXRvcmluZy9LaWxsRmxvd0V2ZW50LmphdmE=) | `0% <0%> (ø)` | `0 <0> (?)` | |
   | [...obblin/service/monitoring/FlowStatusGenerator.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2862/diff?src=pr&el=tree#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9uaXRvcmluZy9GbG93U3RhdHVzR2VuZXJhdG9yLmphdmE=) | `69.69% <0%> (-4.5%)` | `11 <0> (ø)` | |
   | [...org/apache/gobblin/service/FlowStatusResource.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2862/diff?src=pr&el=tree#diff-Z29iYmxpbi1yZXN0bGkvZ29iYmxpbi1mbG93LWNvbmZpZy1zZXJ2aWNlL2dvYmJsaW4tZmxvdy1jb25maWctc2VydmljZS1zZXJ2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vc2VydmljZS9GbG93U3RhdHVzUmVzb3VyY2UuamF2YQ==) | `0% <0%> (ø)` | `0 <0> (ø)` | :arrow_down: |
   | [...rg/apache/gobblin/service/FlowExecutionClient.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2862/diff?src=pr&el=tree#diff-Z29iYmxpbi1yZXN0bGkvZ29iYmxpbi1mbG93LWNvbmZpZy1zZXJ2aWNlL2dvYmJsaW4tZmxvdy1jb25maWctc2VydmljZS1jbGllbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vc2VydmljZS9GbG93RXhlY3V0aW9uQ2xpZW50LmphdmE=) | `0% <0%> (ø)` | `0 <0> (?)` | |
   | [.../apache/gobblin/service/FlowExecutionResource.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2862/diff?src=pr&el=tree#diff-Z29iYmxpbi1yZXN0bGkvZ29iYmxpbi1mbG93LWNvbmZpZy1zZXJ2aWNlL2dvYmJsaW4tZmxvdy1jb25maWctc2VydmljZS1zZXJ2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vc2VydmljZS9GbG93RXhlY3V0aW9uUmVzb3VyY2UuamF2YQ==) | `0% <0%> (ø)` | `0 <0> (?)` | |
   | [...blin/service/modules/orchestration/DagManager.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2862/diff?src=pr&el=tree#diff-Z29iYmxpbi1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9kdWxlcy9vcmNoZXN0cmF0aW9uL0RhZ01hbmFnZXIuamF2YQ==) | `78.23% <50%> (-1.13%)` | `13 <1> (+1)` | |
   | [...in/service/modules/core/GobblinServiceManager.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2862/diff?src=pr&el=tree#diff-Z29iYmxpbi1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9kdWxlcy9jb3JlL0dvYmJsaW5TZXJ2aWNlTWFuYWdlci5qYXZh) | `54.71% <50%> (-0.04%)` | `25 <0> (ø)` | |
   | [...in/java/org/apache/gobblin/cluster/HelixUtils.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2862/diff?src=pr&el=tree#diff-Z29iYmxpbi1jbHVzdGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2NsdXN0ZXIvSGVsaXhVdGlscy5qYXZh) | `32.71% <0%> (-2.81%)` | `11% <0%> (-1%)` | |
   | [.../org/apache/gobblin/metrics/RootMetricContext.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2862/diff?src=pr&el=tree#diff-Z29iYmxpbi1tZXRyaWNzLWxpYnMvZ29iYmxpbi1tZXRyaWNzLWJhc2Uvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vbWV0cmljcy9Sb290TWV0cmljQ29udGV4dC5qYXZh) | `78.12% <0%> (-1.57%)` | `15% <0%> (-1%)` | |
   | ... and [47 more](https://codecov.io/gh/apache/incubator-gobblin/pull/2862/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2862?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/2862?src=pr&el=footer). Last update [d18783d...64f7a43](https://codecov.io/gh/apache/incubator-gobblin/pull/2862?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


With regards,
Apache Git Services

[GitHub] [incubator-gobblin] sv2000 commented on a change in pull request #2862: [GOBBLIN-1017] Deprecate FlowStatus in favor of FlowExecution and add endpoint to kill flows

Posted by GitBox <gi...@apache.org>.
sv2000 commented on a change in pull request #2862: [GOBBLIN-1017] Deprecate FlowStatus in favor of FlowExecution and add endpoint to kill flows
URL: https://github.com/apache/incubator-gobblin/pull/2862#discussion_r371851363
 
 

 ##########
 File path: gobblin-restli/gobblin-flow-config-service/gobblin-flow-config-service-api/src/main/snapshot/org.apache.gobblin.service.flowstatuses.snapshot.json
 ##########
 @@ -38,14 +38,14 @@
     "doc" : "Execution status for a flow or job",
     "symbols" : [ "COMPILED", "PENDING", "PENDING_RETRY", "ORCHESTRATED", "RUNNING", "COMPLETE", "FAILED", "CANCELLED" ],
     "symbolDocs" : {
-      "COMPILED":"Flow compiled to jobs.",
-      "PENDING":"Flow or job is in pending state.",
-      "PENDING_RETRY":"Flow or job is pending retry.",
-      "ORCHESTRATED":"Job(s) orchestrated to spec executors.",
-      "RUNNING": "Flow or job is currently executing",
-      "COMPLETE":"Flow or job completed execution",
-      "FAILED":"Flow or job failed",
-      "CANCELLED":"Flow cancelled."
+      "COMPILED" : "Flow compiled to jobs.",
 
 Review comment:
   Are the whitespaces intended?

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [incubator-gobblin] jack-moseley commented on issue #2862: [GOBBLIN-1017] Deprecate FlowStatus in favor of FlowExecution and add endpoint to kill flows

Posted by GitBox <gi...@apache.org>.
jack-moseley commented on issue #2862: [GOBBLIN-1017] Deprecate FlowStatus in favor of FlowExecution and add endpoint to kill flows
URL: https://github.com/apache/incubator-gobblin/pull/2862#issuecomment-577902329
 
 
   @sv2000 please review

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [incubator-gobblin] codecov-io edited a comment on issue #2862: [GOBBLIN-1017] Deprecate FlowStatus in favor of FlowExecution and add endpoint to kill flows

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #2862: [GOBBLIN-1017] Deprecate FlowStatus in favor of FlowExecution and add endpoint to kill flows
URL: https://github.com/apache/incubator-gobblin/pull/2862#issuecomment-572733455
 
 
   # [Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2862?src=pr&el=h1) Report
   > Merging [#2862](https://codecov.io/gh/apache/incubator-gobblin/pull/2862?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-gobblin/commit/cd53dcfa2c046f53db29689e1d687bf4412565aa?src=pr&el=desc) will **decrease** coverage by `0.05%`.
   > The diff coverage is `4.92%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-gobblin/pull/2862/graphs/tree.svg?width=650&token=4MgURJ0bGc&height=150&src=pr)](https://codecov.io/gh/apache/incubator-gobblin/pull/2862?src=pr&el=tree)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #2862      +/-   ##
   ============================================
   - Coverage      45.8%   45.74%   -0.06%     
   + Complexity     9111     9110       -1     
   ============================================
     Files          1915     1918       +3     
     Lines         72285    72366      +81     
     Branches       7972     7977       +5     
   ============================================
   - Hits          33109    33105       -4     
   - Misses        36150    36234      +84     
   - Partials       3026     3027       +1
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-gobblin/pull/2862?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | [...a/org/apache/gobblin/service/FlowStatusClient.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2862/diff?src=pr&el=tree#diff-Z29iYmxpbi1yZXN0bGkvZ29iYmxpbi1mbG93LWNvbmZpZy1zZXJ2aWNlL2dvYmJsaW4tZmxvdy1jb25maWctc2VydmljZS1jbGllbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vc2VydmljZS9GbG93U3RhdHVzQ2xpZW50LmphdmE=) | `72.34% <ø> (ø)` | `7 <0> (ø)` | :arrow_down: |
   | [...ache/gobblin/service/monitoring/KillFlowEvent.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2862/diff?src=pr&el=tree#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9uaXRvcmluZy9LaWxsRmxvd0V2ZW50LmphdmE=) | `0% <0%> (ø)` | `0 <0> (?)` | |
   | [...obblin/service/monitoring/FlowStatusGenerator.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2862/diff?src=pr&el=tree#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9uaXRvcmluZy9GbG93U3RhdHVzR2VuZXJhdG9yLmphdmE=) | `69.69% <0%> (-4.5%)` | `11 <0> (ø)` | |
   | [...org/apache/gobblin/service/FlowStatusResource.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2862/diff?src=pr&el=tree#diff-Z29iYmxpbi1yZXN0bGkvZ29iYmxpbi1mbG93LWNvbmZpZy1zZXJ2aWNlL2dvYmJsaW4tZmxvdy1jb25maWctc2VydmljZS1zZXJ2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vc2VydmljZS9GbG93U3RhdHVzUmVzb3VyY2UuamF2YQ==) | `0% <0%> (ø)` | `0 <0> (ø)` | :arrow_down: |
   | [...rg/apache/gobblin/service/FlowExecutionClient.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2862/diff?src=pr&el=tree#diff-Z29iYmxpbi1yZXN0bGkvZ29iYmxpbi1mbG93LWNvbmZpZy1zZXJ2aWNlL2dvYmJsaW4tZmxvdy1jb25maWctc2VydmljZS1jbGllbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vc2VydmljZS9GbG93RXhlY3V0aW9uQ2xpZW50LmphdmE=) | `0% <0%> (ø)` | `0 <0> (?)` | |
   | [.../apache/gobblin/service/FlowExecutionResource.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2862/diff?src=pr&el=tree#diff-Z29iYmxpbi1yZXN0bGkvZ29iYmxpbi1mbG93LWNvbmZpZy1zZXJ2aWNlL2dvYmJsaW4tZmxvdy1jb25maWctc2VydmljZS1zZXJ2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vc2VydmljZS9GbG93RXhlY3V0aW9uUmVzb3VyY2UuamF2YQ==) | `0% <0%> (ø)` | `0 <0> (?)` | |
   | [...blin/service/modules/orchestration/DagManager.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2862/diff?src=pr&el=tree#diff-Z29iYmxpbi1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9kdWxlcy9vcmNoZXN0cmF0aW9uL0RhZ01hbmFnZXIuamF2YQ==) | `76.54% <50%> (-1.04%)` | `13 <1> (+1)` | |
   | [...in/service/modules/core/GobblinServiceManager.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2862/diff?src=pr&el=tree#diff-Z29iYmxpbi1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9kdWxlcy9jb3JlL0dvYmJsaW5TZXJ2aWNlTWFuYWdlci5qYXZh) | `54.71% <50%> (-0.04%)` | `25 <0> (ø)` | |
   | [...in/java/org/apache/gobblin/cluster/HelixUtils.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2862/diff?src=pr&el=tree#diff-Z29iYmxpbi1jbHVzdGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2NsdXN0ZXIvSGVsaXhVdGlscy5qYXZh) | `32.71% <0%> (-6.55%)` | `11% <0%> (-2%)` | |
   | [...lin/restli/throttling/ZookeeperLeaderElection.java](https://codecov.io/gh/apache/incubator-gobblin/pull/2862/diff?src=pr&el=tree#diff-Z29iYmxpbi1yZXN0bGkvZ29iYmxpbi10aHJvdHRsaW5nLXNlcnZpY2UvZ29iYmxpbi10aHJvdHRsaW5nLXNlcnZpY2Utc2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3Jlc3RsaS90aHJvdHRsaW5nL1pvb2tlZXBlckxlYWRlckVsZWN0aW9uLmphdmE=) | `70% <0%> (-2.23%)` | `13% <0%> (ø)` | |
   | ... and [7 more](https://codecov.io/gh/apache/incubator-gobblin/pull/2862/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-gobblin/pull/2862?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/2862?src=pr&el=footer). Last update [cd53dcf...8edd06e](https://codecov.io/gh/apache/incubator-gobblin/pull/2862?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


With regards,
Apache Git Services

[GitHub] [incubator-gobblin] jack-moseley commented on a change in pull request #2862: [GOBBLIN-1017] Deprecate FlowStatus in favor of FlowExecution and add endpoint to kill flows

Posted by GitBox <gi...@apache.org>.
jack-moseley commented on a change in pull request #2862: [GOBBLIN-1017] Deprecate FlowStatus in favor of FlowExecution and add endpoint to kill flows
URL: https://github.com/apache/incubator-gobblin/pull/2862#discussion_r371988488
 
 

 ##########
 File path: gobblin-restli/gobblin-flow-config-service/gobblin-flow-config-service-api/src/main/snapshot/org.apache.gobblin.service.flowstatuses.snapshot.json
 ##########
 @@ -38,14 +38,14 @@
     "doc" : "Execution status for a flow or job",
     "symbols" : [ "COMPILED", "PENDING", "PENDING_RETRY", "ORCHESTRATED", "RUNNING", "COMPLETE", "FAILED", "CANCELLED" ],
     "symbolDocs" : {
-      "COMPILED":"Flow compiled to jobs.",
-      "PENDING":"Flow or job is in pending state.",
-      "PENDING_RETRY":"Flow or job is pending retry.",
-      "ORCHESTRATED":"Job(s) orchestrated to spec executors.",
-      "RUNNING": "Flow or job is currently executing",
-      "COMPLETE":"Flow or job completed execution",
-      "FAILED":"Flow or job failed",
-      "CANCELLED":"Flow cancelled."
+      "COMPILED" : "Flow compiled to jobs.",
 
 Review comment:
   Yeah, it generates the files with those spaces so makes it consistent with the other files.

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [incubator-gobblin] sv2000 commented on a change in pull request #2862: [GOBBLIN-1017] Deprecate FlowStatus in favor of FlowExecution and add endpoint to kill flows

Posted by GitBox <gi...@apache.org>.
sv2000 commented on a change in pull request #2862: [GOBBLIN-1017] Deprecate FlowStatus in favor of FlowExecution and add endpoint to kill flows
URL: https://github.com/apache/incubator-gobblin/pull/2862#discussion_r371853777
 
 

 ##########
 File path: gobblin-restli/gobblin-flow-config-service/gobblin-flow-config-service-client/src/main/java/org/apache/gobblin/service/FlowExecutionClient.java
 ##########
 @@ -0,0 +1,170 @@
+/*
+ * 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;
+
+import java.io.Closeable;
+import java.io.IOException;
+import java.util.Collections;
+import java.util.List;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.common.base.Optional;
+import com.google.common.base.Preconditions;
+import com.linkedin.common.callback.FutureCallback;
+import com.linkedin.common.util.None;
+import com.linkedin.r2.RemoteInvocationException;
+import com.linkedin.r2.transport.common.Client;
+import com.linkedin.r2.transport.common.bridge.client.TransportClientAdapter;
+import com.linkedin.r2.transport.http.client.HttpClientFactory;
+import com.linkedin.restli.client.FindRequest;
+import com.linkedin.restli.client.GetRequest;
+import com.linkedin.restli.client.Response;
+import com.linkedin.restli.client.RestClient;
+import com.linkedin.restli.common.CollectionResponse;
+import com.linkedin.restli.common.ComplexResourceKey;
+import com.linkedin.restli.common.EmptyRecord;
+
+
+/**
+ * Flow execution client for REST flow execution server
+ */
+public class FlowExecutionClient implements Closeable {
+  private static final Logger LOG = LoggerFactory.getLogger(FlowExecutionClient.class);
+
+  private Optional<HttpClientFactory> _httpClientFactory;
+  private Optional<RestClient> _restClient;
+  private final FlowexecutionsRequestBuilders _flowexecutionsRequestBuilders;
+
+  /**
+   * Construct a {@link FlowExecutionClient} to communicate with http flow execution server at URI serverUri
+   * @param serverUri address and port of the REST server
+   */
+  public FlowExecutionClient(String serverUri) {
+    LOG.debug("FlowConfigClient with serverUri " + serverUri);
+
+    _httpClientFactory = Optional.of(new HttpClientFactory());
+    Client r2Client = new TransportClientAdapter(_httpClientFactory.get().getClient(Collections.<String, String>emptyMap()));
+    _restClient = Optional.of(new RestClient(r2Client, serverUri));
+
+    _flowexecutionsRequestBuilders = createRequestBuilders();
+  }
+
+  /**
+   * Construct a {@link FlowExecutionClient} to communicate with http flow execution server at URI serverUri
+   * @param restClient restClient to send restli request
+   */
+  public FlowExecutionClient(RestClient restClient) {
+    LOG.debug("FlowConfigClient with restClient " + restClient);
+
+    _httpClientFactory = Optional.absent();
+    _restClient = Optional.of(restClient);
+
+    _flowexecutionsRequestBuilders = createRequestBuilders();
+  }
+
+  protected FlowexecutionsRequestBuilders createRequestBuilders() {
+    return new FlowexecutionsRequestBuilders();
+  }
+
+  /**
+   * Get a flow execution
+   * @param flowStatusId identifier of flow execution to get
+   * @return a {@link FlowExecution} with the flow execution
+   * @throws RemoteInvocationException
+   */
+  public FlowExecution getFlowExecution(FlowStatusId flowStatusId)
+      throws RemoteInvocationException {
+    LOG.debug("getFlowConfig with groupName " + flowStatusId.getFlowGroup() + " flowName " +
+        flowStatusId.getFlowName());
+
+    GetRequest<FlowExecution> getRequest = _flowexecutionsRequestBuilders.get()
+        .id(new ComplexResourceKey<>(flowStatusId, new EmptyRecord())).build();
+
+    Response<FlowExecution> response =
+        _restClient.get().sendRequest(getRequest).getResponse();
+    return response.getEntity();
+  }
+
+  /**
+   * Get the latest flow execution
+   * @param flowId identifier of flow execution to get
+   * @return a {@link FlowExecution}
+   * @throws RemoteInvocationException
+   */
+  public FlowExecution getLatestFlowExecution(FlowId flowId)
+      throws RemoteInvocationException {
+    LOG.debug("getFlowConfig with groupName " + flowId.getFlowGroup() + " flowName " +
+        flowId.getFlowName());
+
+    FindRequest<FlowExecution> findRequest = _flowexecutionsRequestBuilders.findByLatestFlowExecution().flowIdParam(flowId).build();
+
+    Response<CollectionResponse<FlowExecution>> response =
+        _restClient.get().sendRequest(findRequest).getResponse();
+
+    List<FlowExecution> flowExecutionList = response.getEntity().getElements();
 
 Review comment:
   Is a null check needed here?

----------------------------------------------------------------
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


With regards,
Apache Git Services