You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@submarine.apache.org by GitBox <gi...@apache.org> on 2020/04/15 14:31:22 UTC

[GitHub] [submarine] JohnTing opened a new pull request #263: SUBMARINE-442. Support get job's log in submarine-server REST API

JohnTing opened a new pull request #263: SUBMARINE-442. Support get job's log in submarine-server REST API
URL: https://github.com/apache/submarine/pull/263
 
 
   ### What is this PR for?
   
   Now we have the "jobs" resource in REST which can do CRUD. We also need a "logs" API to get the job's log output. The URI could be "api/v1/logs"
   It should accept parameters like "jobid". Initially, the logs could be aggregated logs of all containers.
   Streaming is preferred so that the python client can enable a fancy way for the end-user to check logs
   
   ### What type of PR is it?
   Feature
   
   ### Todos
   * [X] - get logs so far
   * [X] - get logs until the end of the pod
   
   ### What is the Jira issue?
   https://issues.apache.org/jira/projects/SUBMARINE/issues/SUBMARINE-442
   
   ### How should this be tested?
   Create a job 
   Visit /api/v1/logs/{jobid} or /api/v1/logs/stream/{jobid} with a browser
   
   ### Screenshots (if appropriate)
   http://127.0.0.1:8080/api/v1/logs/job_1586957273819_0001
   ![image](https://user-images.githubusercontent.com/19265751/79346001-69667180-7f64-11ea-97e9-227565e76a91.png)
   
   http://127.0.0.1:8080/api/v1/logs/stream/job_1586957273819_0001
   ![image](https://user-images.githubusercontent.com/19265751/79346035-73887000-7f64-11ea-86cb-125504028087.png)
   
   
   ### Questions:
   * Does the licenses files need update? No
   * Is there breaking changes for older versions? No
   * Does this needs documentation? No

----------------------------------------------------------------
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] [submarine] tangzhankun commented on a change in pull request #263: SUBMARINE-442. Support get job's log in submarine-server REST API

Posted by GitBox <gi...@apache.org>.
tangzhankun commented on a change in pull request #263: SUBMARINE-442. Support get job's log in submarine-server REST API
URL: https://github.com/apache/submarine/pull/263#discussion_r409270029
 
 

 ##########
 File path: submarine-server/server-submitter/submitter-k8s/src/main/java/org/apache/submarine/server/submitter/k8s/K8sJobSubmitter.java
 ##########
 @@ -186,6 +197,80 @@ private Job parseResponseObject(Object object, ParseOp op) throws SubmarineRunti
     throw new SubmarineRuntimeException(500, "K8s Submitter parse upstream response failed.");
   }
 
+  public String getLog(final Job job) {
+    if (job == null) {
+      return null;
+    }
+    final CoreV1Api COREV1_API = new CoreV1Api(client);
+
+    try {
+      final V1PodList podList = COREV1_API.listNamespacedPod(
+          job.getSpec().getSubmitterSpec().getNamespace(),
+          "false", null, null,
+          getJobLabelSelector(job), null, null,
+          null, null);
+
+      final V1Pod pod = podList.getItems().get(0);
 
 Review comment:
   It would be several containers in the "podList", here it only gets the first one. Is it better that we merge all container's pod with a format? Like this:
   {
   jobId: job_123,
   jobType: tensorflow,
   LogContent: [
   {podName: tf-ps-0,
   podLog: 123123123},
   {podName: tf-worker-0,
   podLog: 456456456},
   ...
   ]
   }
   

----------------------------------------------------------------
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] [submarine] pingsutw commented on a change in pull request #263: SUBMARINE-442. Support get job's log in submarine-server REST API

Posted by GitBox <gi...@apache.org>.
pingsutw commented on a change in pull request #263: SUBMARINE-442. Support get job's log in submarine-server REST API
URL: https://github.com/apache/submarine/pull/263#discussion_r410233867
 
 

 ##########
 File path: submarine-server/server-api/src/main/java/org/apache/submarine/server/api/job/JobLog.java
 ##########
 @@ -0,0 +1,43 @@
+/*
+ * 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.submarine.server.api.job;
+
+// import java.io.InputStream;
 
 Review comment:
   Need to remove unused import

----------------------------------------------------------------
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] [submarine] yuanzac commented on issue #263: SUBMARINE-442. Support get job's log in submarine-server REST API

Posted by GitBox <gi...@apache.org>.
yuanzac commented on issue #263: SUBMARINE-442. Support get job's log in submarine-server REST API
URL: https://github.com/apache/submarine/pull/263#issuecomment-615838153
 
 
   Trigger the travis ci again.

----------------------------------------------------------------
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] [submarine] pingsutw commented on a change in pull request #263: SUBMARINE-442. Support get job's log in submarine-server REST API

Posted by GitBox <gi...@apache.org>.
pingsutw commented on a change in pull request #263: SUBMARINE-442. Support get job's log in submarine-server REST API
URL: https://github.com/apache/submarine/pull/263#discussion_r409319566
 
 

 ##########
 File path: submarine-server/server-submitter/submitter-k8s/src/main/java/org/apache/submarine/server/submitter/k8s/K8sJobSubmitter.java
 ##########
 @@ -186,6 +197,80 @@ private Job parseResponseObject(Object object, ParseOp op) throws SubmarineRunti
     throw new SubmarineRuntimeException(500, "K8s Submitter parse upstream response failed.");
   }
 
+  public String getLog(final Job job) {
+    if (job == null) {
+      return null;
+    }
+    final CoreV1Api COREV1_API = new CoreV1Api(client);
+
+    try {
+      final V1PodList podList = COREV1_API.listNamespacedPod(
+          job.getSpec().getSubmitterSpec().getNamespace(),
+          "false", null, null,
+          getJobLabelSelector(job), null, null,
+          null, null);
+
+      final V1Pod pod = podList.getItems().get(0);
+      final String podName = pod.getMetadata().getName();
+      final String namespace = job.getSpec().getSubmitterSpec().getNamespace();
+      
+      String readNamespacedPodLog =
+          COREV1_API.readNamespacedPodLog(
+            podName,
+            namespace,
+            null,
+            Boolean.FALSE,
+            Integer.MAX_VALUE,
+            null,
+            Boolean.FALSE,
+            Integer.MAX_VALUE,
+            40,
 
 Review comment:
   We should use a constant variable instead of hardcode `40`.

----------------------------------------------------------------
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] [submarine] liuxunorg commented on a change in pull request #263: SUBMARINE-442. Support get job's log in submarine-server REST API

Posted by GitBox <gi...@apache.org>.
liuxunorg commented on a change in pull request #263: SUBMARINE-442. Support get job's log in submarine-server REST API
URL: https://github.com/apache/submarine/pull/263#discussion_r410692593
 
 

 ##########
 File path: submarine-server/server-core/src/main/java/org/apache/submarine/server/rest/JobManagerRestApi.java
 ##########
 @@ -132,6 +134,32 @@ public Response deleteJob(@PathParam(RestConstants.JOB_ID) String id) {
       return parseJobServiceException(e);
     }
   }
+  
+  @GET
+  @Path("/logs")
+  public Response listlog(@QueryParam("status") String status) {
+    try {
+      Map<String, List<JobLog>> jobLogList = jobManager.listJobLogsByStatus(status);
+      return new JsonResponse.Builder<Map<String, List<JobLog>>>(Response.Status.OK).
+          result(jobLogList).build();
+
+    } catch (SubmarineRuntimeException e) {
+      return parseJobServiceException(e);
+    }
+  }
+
+  @GET
+  @Path("/logs/{id}")
+  public Response getlog(@PathParam(RestConstants.JOB_ID) String id) {
 
 Review comment:
   The hump command method should be used.
   Need change to `getLog(...`

----------------------------------------------------------------
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] [submarine] JohnTing commented on a change in pull request #263: SUBMARINE-442. Support get job's log in submarine-server REST API

Posted by GitBox <gi...@apache.org>.
JohnTing commented on a change in pull request #263: SUBMARINE-442. Support get job's log in submarine-server REST API
URL: https://github.com/apache/submarine/pull/263#discussion_r409296239
 
 

 ##########
 File path: submarine-server/server-core/src/test/java/org/apache/submarine/server/rest/JobLogRestApiTest.java
 ##########
 @@ -0,0 +1,53 @@
+/*
+ * 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.submarine.server.rest;
+
+import org.apache.commons.httpclient.methods.GetMethod;
+import org.apache.submarine.server.AbstractSubmarineServerTest;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+
+public class JobLogRestApiTest extends AbstractSubmarineServerTest {
+
+  private static final Logger LOG = LoggerFactory.getLogger(JobLogRestApiTest.class);
+
+  @BeforeClass
+  public static void init() throws Exception {
+    AbstractSubmarineServerTest.startUp(JobLogRestApiTest.class.getSimpleName());
+  }
+
+  @AfterClass
+  public static void destroy() throws Exception {
+    AbstractSubmarineServerTest.shutDown();
+  }
+
+  @Test
+  public void TestGetJobLogByJobId() throws IOException {
+    String jobId = "job_1586957273819_0001";
+    GetMethod response = httpGet("/api/" + RestConstants.V1 + "/"
+        + RestConstants.LOGS + "/" + jobId);
+    LOG.info(response.toString());
 
 Review comment:
   Ok, I will add it

----------------------------------------------------------------
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] [submarine] JohnTing commented on a change in pull request #263: SUBMARINE-442. Support get job's log in submarine-server REST API

Posted by GitBox <gi...@apache.org>.
JohnTing commented on a change in pull request #263: SUBMARINE-442. Support get job's log in submarine-server REST API
URL: https://github.com/apache/submarine/pull/263#discussion_r409295706
 
 

 ##########
 File path: submarine-server/server-submitter/submitter-k8s/src/main/java/org/apache/submarine/server/submitter/k8s/K8sJobSubmitter.java
 ##########
 @@ -186,6 +197,80 @@ private Job parseResponseObject(Object object, ParseOp op) throws SubmarineRunti
     throw new SubmarineRuntimeException(500, "K8s Submitter parse upstream response failed.");
   }
 
+  public String getLog(final Job job) {
+    if (job == null) {
+      return null;
+    }
+    final CoreV1Api COREV1_API = new CoreV1Api(client);
+
+    try {
+      final V1PodList podList = COREV1_API.listNamespacedPod(
+          job.getSpec().getSubmitterSpec().getNamespace(),
+          "false", null, null,
+          getJobLabelSelector(job), null, null,
+          null, null);
+
+      final V1Pod pod = podList.getItems().get(0);
 
 Review comment:
   Ok, I will modify it

----------------------------------------------------------------
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] [submarine] pingsutw commented on a change in pull request #263: SUBMARINE-442. Support get job's log in submarine-server REST API

Posted by GitBox <gi...@apache.org>.
pingsutw commented on a change in pull request #263: SUBMARINE-442. Support get job's log in submarine-server REST API
URL: https://github.com/apache/submarine/pull/263#discussion_r409320560
 
 

 ##########
 File path: submarine-server/server-submitter/submitter-k8s/src/main/java/org/apache/submarine/server/submitter/k8s/K8sJobSubmitter.java
 ##########
 @@ -186,6 +197,80 @@ private Job parseResponseObject(Object object, ParseOp op) throws SubmarineRunti
     throw new SubmarineRuntimeException(500, "K8s Submitter parse upstream response failed.");
   }
 
+  public String getLog(final Job job) {
+    if (job == null) {
+      return null;
+    }
+    final CoreV1Api COREV1_API = new CoreV1Api(client);
+
+    try {
+      final V1PodList podList = COREV1_API.listNamespacedPod(
+          job.getSpec().getSubmitterSpec().getNamespace(),
+          "false", null, null,
+          getJobLabelSelector(job), null, null,
+          null, null);
+
+      final V1Pod pod = podList.getItems().get(0);
+      final String podName = pod.getMetadata().getName();
+      final String namespace = job.getSpec().getSubmitterSpec().getNamespace();
+      
+      String readNamespacedPodLog =
+          COREV1_API.readNamespacedPodLog(
+            podName,
+            namespace,
+            null,
+            Boolean.FALSE,
+            Integer.MAX_VALUE,
+            null,
+            Boolean.FALSE,
+            Integer.MAX_VALUE,
+            40,
+            Boolean.FALSE);
+
+      return readNamespacedPodLog;
+
+    } catch (final ApiException e) {
+      LOG.warn("Error when listing pod for job:" + job.toString(), e.getMessage());
+    }
+    return null;
+  }
+
+  public InputStream getLogStream(final Job job) {
+    if (job == null) {
+      return null;
+    }
+    final CoreV1Api COREV1_API = new CoreV1Api(client);
+
+    try {
+      final V1PodList podList = COREV1_API.listNamespacedPod(
+          job.getSpec().getSubmitterSpec().getNamespace(),
+          "false", null, null,
+          getJobLabelSelector(job), null, null,
+          null, null);
+      final V1Pod pod = podList.getItems().get(0);
+      PodLogs logs = new PodLogs();
+      
+      InputStream inputStream = logs.streamNamespacedPodLog(pod);
+      return inputStream;
+
+    } catch (final ApiException e) {
+      LOG.warn("Error when listing pod for job:" + job.toString(), e.getMessage());
 
 Review comment:
   ```suggestion
         LOG.error("Error when listing pod for job:" + job.toString(), e.getMessage());
   ```

----------------------------------------------------------------
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] [submarine] JohnTing commented on a change in pull request #263: SUBMARINE-442. Support get job's log in submarine-server REST API

Posted by GitBox <gi...@apache.org>.
JohnTing commented on a change in pull request #263: SUBMARINE-442. Support get job's log in submarine-server REST API
URL: https://github.com/apache/submarine/pull/263#discussion_r409299062
 
 

 ##########
 File path: submarine-server/server-submitter/submitter-k8s/src/main/java/org/apache/submarine/server/submitter/k8s/K8sJobSubmitter.java
 ##########
 @@ -186,6 +197,80 @@ private Job parseResponseObject(Object object, ParseOp op) throws SubmarineRunti
     throw new SubmarineRuntimeException(500, "K8s Submitter parse upstream response failed.");
   }
 
+  public String getLog(final Job job) {
+    if (job == null) {
+      return null;
+    }
+    final CoreV1Api COREV1_API = new CoreV1Api(client);
+
+    try {
+      final V1PodList podList = COREV1_API.listNamespacedPod(
+          job.getSpec().getSubmitterSpec().getNamespace(),
+          "false", null, null,
+          getJobLabelSelector(job), null, null,
+          null, null);
+
+      final V1Pod pod = podList.getItems().get(0);
+      final String podName = pod.getMetadata().getName();
+      final String namespace = job.getSpec().getSubmitterSpec().getNamespace();
+      
+      String readNamespacedPodLog =
+          COREV1_API.readNamespacedPodLog(
+            podName,
+            namespace,
+            null,
+            Boolean.FALSE,
+            Integer.MAX_VALUE,
+            null,
+            Boolean.FALSE,
+            Integer.MAX_VALUE,
+            40,
+            Boolean.FALSE);
+
+      return readNamespacedPodLog;
+
+    } catch (final ApiException e) {
+      LOG.warn("Error when listing pod for job:" + job.toString(), e.getMessage());
+    }
+    return null;
+  }
+
+  public InputStream getLogStream(final Job job) {
+    if (job == null) {
+      return null;
+    }
+    final CoreV1Api COREV1_API = new CoreV1Api(client);
+
+    try {
+      final V1PodList podList = COREV1_API.listNamespacedPod(
+          job.getSpec().getSubmitterSpec().getNamespace(),
+          "false", null, null,
+          getJobLabelSelector(job), null, null,
+          null, null);
+      final V1Pod pod = podList.getItems().get(0);
 
 Review comment:
   I am not sure how to show the mixed stream in the browser ...
   Maybe something like this?
   [
   {"pod1": "log start"},
   {"pod1": "1%"},
   {"pod2": "log start"},
   {"pod1": "2%"},
   {"pod2": "1%"},
   ....
   

----------------------------------------------------------------
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] [submarine] liuxunorg commented on a change in pull request #263: SUBMARINE-442. Support get job's log in submarine-server REST API

Posted by GitBox <gi...@apache.org>.
liuxunorg commented on a change in pull request #263: SUBMARINE-442. Support get job's log in submarine-server REST API
URL: https://github.com/apache/submarine/pull/263#discussion_r410692565
 
 

 ##########
 File path: submarine-server/server-core/src/main/java/org/apache/submarine/server/rest/JobManagerRestApi.java
 ##########
 @@ -132,6 +134,32 @@ public Response deleteJob(@PathParam(RestConstants.JOB_ID) String id) {
       return parseJobServiceException(e);
     }
   }
+  
+  @GET
+  @Path("/logs")
+  public Response listlog(@QueryParam("status") String status) {
 
 Review comment:
   The hump command method should be used.
   Need change to `listLog(...`

----------------------------------------------------------------
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] [submarine] pingsutw commented on a change in pull request #263: SUBMARINE-442. Support get job's log in submarine-server REST API

Posted by GitBox <gi...@apache.org>.
pingsutw commented on a change in pull request #263: SUBMARINE-442. Support get job's log in submarine-server REST API
URL: https://github.com/apache/submarine/pull/263#discussion_r409320682
 
 

 ##########
 File path: submarine-server/server-submitter/submitter-k8s/src/main/java/org/apache/submarine/server/submitter/k8s/K8sJobSubmitter.java
 ##########
 @@ -186,6 +197,80 @@ private Job parseResponseObject(Object object, ParseOp op) throws SubmarineRunti
     throw new SubmarineRuntimeException(500, "K8s Submitter parse upstream response failed.");
   }
 
+  public String getLog(final Job job) {
+    if (job == null) {
+      return null;
+    }
+    final CoreV1Api COREV1_API = new CoreV1Api(client);
+
+    try {
+      final V1PodList podList = COREV1_API.listNamespacedPod(
+          job.getSpec().getSubmitterSpec().getNamespace(),
+          "false", null, null,
+          getJobLabelSelector(job), null, null,
+          null, null);
+
+      final V1Pod pod = podList.getItems().get(0);
+      final String podName = pod.getMetadata().getName();
+      final String namespace = job.getSpec().getSubmitterSpec().getNamespace();
+      
+      String readNamespacedPodLog =
+          COREV1_API.readNamespacedPodLog(
+            podName,
+            namespace,
+            null,
+            Boolean.FALSE,
+            Integer.MAX_VALUE,
+            null,
+            Boolean.FALSE,
+            Integer.MAX_VALUE,
+            40,
+            Boolean.FALSE);
+
+      return readNamespacedPodLog;
+
+    } catch (final ApiException e) {
+      LOG.warn("Error when listing pod for job:" + job.toString(), e.getMessage());
+    }
+    return null;
+  }
+
+  public InputStream getLogStream(final Job job) {
+    if (job == null) {
+      return null;
+    }
+    final CoreV1Api COREV1_API = new CoreV1Api(client);
+
+    try {
+      final V1PodList podList = COREV1_API.listNamespacedPod(
+          job.getSpec().getSubmitterSpec().getNamespace(),
+          "false", null, null,
+          getJobLabelSelector(job), null, null,
+          null, null);
+      final V1Pod pod = podList.getItems().get(0);
+      PodLogs logs = new PodLogs();
+      
+      InputStream inputStream = logs.streamNamespacedPodLog(pod);
+      return inputStream;
+
+    } catch (final ApiException e) {
+      LOG.warn("Error when listing pod for job:" + job.toString(), e.getMessage());
+    } catch (final IOException e) {
+      LOG.warn("Error when get pod log stream", e.getMessage());
 
 Review comment:
   ```suggestion
         LOG.error("Error when get pod log stream", e.getMessage());
   ```

----------------------------------------------------------------
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] [submarine] tangzhankun commented on a change in pull request #263: SUBMARINE-442. Support get job's log in submarine-server REST API

Posted by GitBox <gi...@apache.org>.
tangzhankun commented on a change in pull request #263: SUBMARINE-442. Support get job's log in submarine-server REST API
URL: https://github.com/apache/submarine/pull/263#discussion_r409270837
 
 

 ##########
 File path: submarine-server/server-submitter/submitter-k8s/src/main/java/org/apache/submarine/server/submitter/k8s/K8sJobSubmitter.java
 ##########
 @@ -186,6 +197,80 @@ private Job parseResponseObject(Object object, ParseOp op) throws SubmarineRunti
     throw new SubmarineRuntimeException(500, "K8s Submitter parse upstream response failed.");
   }
 
+  public String getLog(final Job job) {
+    if (job == null) {
+      return null;
+    }
+    final CoreV1Api COREV1_API = new CoreV1Api(client);
+
+    try {
+      final V1PodList podList = COREV1_API.listNamespacedPod(
+          job.getSpec().getSubmitterSpec().getNamespace(),
+          "false", null, null,
+          getJobLabelSelector(job), null, null,
+          null, null);
+
+      final V1Pod pod = podList.getItems().get(0);
+      final String podName = pod.getMetadata().getName();
+      final String namespace = job.getSpec().getSubmitterSpec().getNamespace();
+      
+      String readNamespacedPodLog =
+          COREV1_API.readNamespacedPodLog(
+            podName,
+            namespace,
+            null,
+            Boolean.FALSE,
+            Integer.MAX_VALUE,
+            null,
+            Boolean.FALSE,
+            Integer.MAX_VALUE,
+            40,
+            Boolean.FALSE);
+
+      return readNamespacedPodLog;
+
+    } catch (final ApiException e) {
+      LOG.warn("Error when listing pod for job:" + job.toString(), e.getMessage());
+    }
+    return null;
+  }
+
+  public InputStream getLogStream(final Job job) {
+    if (job == null) {
+      return null;
+    }
+    final CoreV1Api COREV1_API = new CoreV1Api(client);
+
+    try {
+      final V1PodList podList = COREV1_API.listNamespacedPod(
+          job.getSpec().getSubmitterSpec().getNamespace(),
+          "false", null, null,
+          getJobLabelSelector(job), null, null,
+          null, null);
+      final V1Pod pod = podList.getItems().get(0);
 
 Review comment:
   Also, is it possible that we merge different streams into one? And update the aggregated log stream?

----------------------------------------------------------------
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] [submarine] JohnTing commented on issue #263: SUBMARINE-442. Support get job's log in submarine-server REST API

Posted by GitBox <gi...@apache.org>.
JohnTing commented on issue #263: SUBMARINE-442. Support get job's log in submarine-server REST API
URL: https://github.com/apache/submarine/pull/263#issuecomment-615409892
 
 
   @tangzhankun, @jiwq, @pingsutw 
   Thank you for the advice.
   I have updated done. Please help me preview this PR, thanks a lot.

----------------------------------------------------------------
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] [submarine] pingsutw commented on a change in pull request #263: SUBMARINE-442. Support get job's log in submarine-server REST API

Posted by GitBox <gi...@apache.org>.
pingsutw commented on a change in pull request #263: SUBMARINE-442. Support get job's log in submarine-server REST API
URL: https://github.com/apache/submarine/pull/263#discussion_r409319566
 
 

 ##########
 File path: submarine-server/server-submitter/submitter-k8s/src/main/java/org/apache/submarine/server/submitter/k8s/K8sJobSubmitter.java
 ##########
 @@ -186,6 +197,80 @@ private Job parseResponseObject(Object object, ParseOp op) throws SubmarineRunti
     throw new SubmarineRuntimeException(500, "K8s Submitter parse upstream response failed.");
   }
 
+  public String getLog(final Job job) {
+    if (job == null) {
+      return null;
+    }
+    final CoreV1Api COREV1_API = new CoreV1Api(client);
+
+    try {
+      final V1PodList podList = COREV1_API.listNamespacedPod(
+          job.getSpec().getSubmitterSpec().getNamespace(),
+          "false", null, null,
+          getJobLabelSelector(job), null, null,
+          null, null);
+
+      final V1Pod pod = podList.getItems().get(0);
+      final String podName = pod.getMetadata().getName();
+      final String namespace = job.getSpec().getSubmitterSpec().getNamespace();
+      
+      String readNamespacedPodLog =
+          COREV1_API.readNamespacedPodLog(
+            podName,
+            namespace,
+            null,
+            Boolean.FALSE,
+            Integer.MAX_VALUE,
+            null,
+            Boolean.FALSE,
+            Integer.MAX_VALUE,
+            40,
 
 Review comment:
   We should use a constant value instead of hardcode `40`.

----------------------------------------------------------------
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] [submarine] liuxunorg commented on issue #263: SUBMARINE-442. Support get job's log in submarine-server REST API

Posted by GitBox <gi...@apache.org>.
liuxunorg commented on issue #263: SUBMARINE-442. Support get job's log in submarine-server REST API
URL: https://github.com/apache/submarine/pull/263#issuecomment-615864505
 
 
   @JohnTing You need to add test cases to ensure the correctness of your code. It can also ensure that the code you submit will not be modified by others.
   like this, https://github.com/apache/submarine/blob/master/submarine-test/test-k8s/src/test/java/org/apache/submarine/rest/JobManagerRestApiIT.java#L134

----------------------------------------------------------------
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] [submarine] tangzhankun commented on a change in pull request #263: SUBMARINE-442. Support get job's log in submarine-server REST API

Posted by GitBox <gi...@apache.org>.
tangzhankun commented on a change in pull request #263: SUBMARINE-442. Support get job's log in submarine-server REST API
URL: https://github.com/apache/submarine/pull/263#discussion_r409271620
 
 

 ##########
 File path: submarine-server/server-core/src/test/java/org/apache/submarine/server/rest/JobLogRestApiTest.java
 ##########
 @@ -0,0 +1,53 @@
+/*
+ * 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.submarine.server.rest;
+
+import org.apache.commons.httpclient.methods.GetMethod;
+import org.apache.submarine.server.AbstractSubmarineServerTest;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+
+public class JobLogRestApiTest extends AbstractSubmarineServerTest {
+
+  private static final Logger LOG = LoggerFactory.getLogger(JobLogRestApiTest.class);
+
+  @BeforeClass
+  public static void init() throws Exception {
+    AbstractSubmarineServerTest.startUp(JobLogRestApiTest.class.getSimpleName());
+  }
+
+  @AfterClass
+  public static void destroy() throws Exception {
+    AbstractSubmarineServerTest.shutDown();
+  }
+
+  @Test
+  public void TestGetJobLogByJobId() throws IOException {
+    String jobId = "job_1586957273819_0001";
+    GetMethod response = httpGet("/api/" + RestConstants.V1 + "/"
+        + RestConstants.LOGS + "/" + jobId);
+    LOG.info(response.toString());
 
 Review comment:
   Can we have some assertions in this test case? Usually, if it could run some job and get the job log would be easy. If we cannot do any assertion here. I suggest we remove this end-to-end unit test.

----------------------------------------------------------------
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] [submarine] JohnTing commented on a change in pull request #263: SUBMARINE-442. Support get job's log in submarine-server REST API

Posted by GitBox <gi...@apache.org>.
JohnTing commented on a change in pull request #263: SUBMARINE-442. Support get job's log in submarine-server REST API
URL: https://github.com/apache/submarine/pull/263#discussion_r409299062
 
 

 ##########
 File path: submarine-server/server-submitter/submitter-k8s/src/main/java/org/apache/submarine/server/submitter/k8s/K8sJobSubmitter.java
 ##########
 @@ -186,6 +197,80 @@ private Job parseResponseObject(Object object, ParseOp op) throws SubmarineRunti
     throw new SubmarineRuntimeException(500, "K8s Submitter parse upstream response failed.");
   }
 
+  public String getLog(final Job job) {
+    if (job == null) {
+      return null;
+    }
+    final CoreV1Api COREV1_API = new CoreV1Api(client);
+
+    try {
+      final V1PodList podList = COREV1_API.listNamespacedPod(
+          job.getSpec().getSubmitterSpec().getNamespace(),
+          "false", null, null,
+          getJobLabelSelector(job), null, null,
+          null, null);
+
+      final V1Pod pod = podList.getItems().get(0);
+      final String podName = pod.getMetadata().getName();
+      final String namespace = job.getSpec().getSubmitterSpec().getNamespace();
+      
+      String readNamespacedPodLog =
+          COREV1_API.readNamespacedPodLog(
+            podName,
+            namespace,
+            null,
+            Boolean.FALSE,
+            Integer.MAX_VALUE,
+            null,
+            Boolean.FALSE,
+            Integer.MAX_VALUE,
+            40,
+            Boolean.FALSE);
+
+      return readNamespacedPodLog;
+
+    } catch (final ApiException e) {
+      LOG.warn("Error when listing pod for job:" + job.toString(), e.getMessage());
+    }
+    return null;
+  }
+
+  public InputStream getLogStream(final Job job) {
+    if (job == null) {
+      return null;
+    }
+    final CoreV1Api COREV1_API = new CoreV1Api(client);
+
+    try {
+      final V1PodList podList = COREV1_API.listNamespacedPod(
+          job.getSpec().getSubmitterSpec().getNamespace(),
+          "false", null, null,
+          getJobLabelSelector(job), null, null,
+          null, null);
+      final V1Pod pod = podList.getItems().get(0);
 
 Review comment:
   I am not sure how to render the mixed stream in the browser ...
   Maybe something like this?
   [
   {"pod1": "log start"},
   {"pod1": "1%"},
   {"pod2": "log start"},
   {"pod1": "2%"},
   {"pod2": "1%"},
   ....
   

----------------------------------------------------------------
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] [submarine] liuxunorg commented on a change in pull request #263: SUBMARINE-442. Support get job's log in submarine-server REST API

Posted by GitBox <gi...@apache.org>.
liuxunorg commented on a change in pull request #263: SUBMARINE-442. Support get job's log in submarine-server REST API
URL: https://github.com/apache/submarine/pull/263#discussion_r410693506
 
 

 ##########
 File path: submarine-server/server-submitter/submitter-k8s/src/main/java/org/apache/submarine/server/submitter/k8s/K8sJobSubmitter.java
 ##########
 @@ -54,9 +61,14 @@
 
   private String confPath;
 
+  private static final String TF_JOB_SELECTOR_KEY = "tf-job-name=";
+  private static final String PYTORCH_JOB_SELECTOR_KEY = "pytorch-job-name=";
+
   // K8s API client for CRD
   private CustomObjectsApi api;
 
+  private CoreV1Api COREV1_API;
 
 Review comment:
   The hump command method should be used.
   Need change to `coreApi`

----------------------------------------------------------------
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] [submarine] pingsutw commented on a change in pull request #263: SUBMARINE-442. Support get job's log in submarine-server REST API

Posted by GitBox <gi...@apache.org>.
pingsutw commented on a change in pull request #263: SUBMARINE-442. Support get job's log in submarine-server REST API
URL: https://github.com/apache/submarine/pull/263#discussion_r409318068
 
 

 ##########
 File path: submarine-server/server-submitter/submitter-k8s/src/main/java/org/apache/submarine/server/submitter/k8s/K8sJobSubmitter.java
 ##########
 @@ -54,9 +60,14 @@
 
   private String confPath;
 
+  private static final String TF_JOB_SELECTOR_KEY = "tf-job-name=";
+  private static final String PYTORCH_JOB_SELECTOR_KEY = "pytorch-job-name=";
+
   // K8s API client for CRD
   private CustomObjectsApi api;
 
+  ApiClient client;
 
 Review comment:
   ```suggestion
     private ApiClient 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


With regards,
Apache Git Services