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/03/25 14:40:51 UTC
[GitHub] [submarine] jiwq opened a new pull request #249: SUBMARINE-455.
Support find/patch/delete job in submarine-server REST
jiwq opened a new pull request #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249
### What is this PR for?
Nowadays the submarine-server only support create the job by REST. We should implement the other operations.
- GET/PATCH/DELETE job by REST
- Fill more info for Job object, such as the job status(accepted/created/running/succeeded) with time and the spec etc.
### What type of PR is it?
Feature
### Todos
### What is the Jira issue?
https://issues.apache.org/jira/browse/SUBMARINE-455
### How should this be tested?
https://travis-ci.com/github/jiwq/submarine/builds/155090047
### Screenshots (if appropriate)
### 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 issue #249: SUBMARINE-455.
Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
tangzhankun commented on issue #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#issuecomment-609788426
Thanks for the contribution Wanqiang, Xun! Since this is a large PR and we have lots of comments. Let's discuss more details in our next sync up.
----------------------------------------------------------------
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 #249:
SUBMARINE-455. Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
liuxunorg commented on a change in pull request #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#discussion_r404023728
##########
File path: submarine-test/test-k8s/src/test/java/org/apache/submarine/server/client/ClusterRestApiIT.java
##########
@@ -0,0 +1,47 @@
+/*
+ * 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.client;
+
+import org.apache.commons.httpclient.methods.GetMethod;
+
+import org.junit.Assert;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.junit.Test;
+
+public class ClusterRestApiIT {
Review comment:
Don't do anything unrelated to this PR
----------------------------------------------------------------
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 #249:
SUBMARINE-455. Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
liuxunorg commented on a change in pull request #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#discussion_r404041266
##########
File path: conf/submarine-site.xml.template
##########
@@ -155,4 +155,28 @@
<description>Rpc server port</description>
</property>
+ <!-- Submarine Submitters Configuration -->
+ <property>
+ <name>submarine.submitters</name>
+ <value>k8s</value>
+ <description>Submitter list for the server</description>
+ </property>
+ <property>
+ <name>submarine.submitters.k8s.class</name>
+ <value>org.apache.submarine.server.submitter.k8s.K8sJobSubmitter</value>
+ <description>The entry class for the specified submitter</description>
+ </property>
+ <property>
+ <name>submarine.submitters.k8s.classpath</name>
+ <value>../lib/submitter/k8s/</value>
+ <description>The libs for Kubernetes submitter</description>
+ </property>
+
+ <!-- K8s Configuration -->
+ <property>
+ <name>submarine.k8s.kube.config</name>
Review comment:
k8s cluster already have `KUBECONFIG`, When testing locally, you can get and set environment variables through `KUBECONFIG`.
----------------------------------------------------------------
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 #249:
SUBMARINE-455. Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
tangzhankun commented on a change in pull request #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#discussion_r399739696
##########
File path: submarine-server/server-core/src/test/java/org/apache/submarine/server/rest/JobManagerRestApiTest.java
##########
@@ -137,16 +134,12 @@ public void testGetJobById() throws IOException {
.create();
JsonResponse<Job> jsonResponse = gson.fromJson(responseBodyAsString,
new TypeToken<JsonResponse<Job>>(){}.getType());
- assertEquals("Response code should be 200 ",
- Response.Status.OK.getStatusCode(), jsonResponse.getCode());
- assertEquals("Job id should be " + jobId, jobId,
- jsonResponse.getResult().getJobId().toString());
+ assertEquals(Response.Status.NOT_FOUND.getStatusCode(), jsonResponse.getCode());
Review comment:
This probably needs to mock the JobManager internals job list beforehand or post jobs specs to ensure the JobManager store it before the query.
@liuxunorg any better idea or help on how to mock the submarine server? The AbstractSubmarineServerTest seems is starting up a real submarine server. To me it seems not easy to mock the submarine server's internals easily.
----------------------------------------------------------------
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 a change in pull request #249:
SUBMARINE-455. Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
yuanzac commented on a change in pull request #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#discussion_r399079228
##########
File path: submarine-server/server-api/src/main/java/org/apache/submarine/server/api/JobHandler.java
##########
@@ -19,53 +19,64 @@
package org.apache.submarine.server.api;
+import java.util.List;
+
import org.apache.submarine.server.api.exception.InvalidSpecException;
-import org.apache.submarine.server.api.exception.UnsupportedJobTypeException;
+import org.apache.submarine.server.api.exception.NotFoundJobException;
+import org.apache.submarine.server.api.exception.UnsupportedSubmitterException;
import org.apache.submarine.server.api.job.Job;
import org.apache.submarine.server.api.spec.JobSpec;
/**
- * Handle the job's operate (CRUD)
+ * Handle the job's operation (CRUD)
*/
public interface JobHandler {
/**
- * Submit job
+ * Create job
* @param jobSpec job spec
- * @return job object
- * @throws UnsupportedJobTypeException caused by the unsupported job type
+ * @return object
+ * @throws UnsupportedSubmitterException unsupported submitter
+ * @throws InvalidSpecException invalid spec
*/
- Job submitJob(JobSpec jobSpec) throws UnsupportedJobTypeException, InvalidSpecException;
+ Job createJob(JobSpec jobSpec) throws UnsupportedSubmitterException, InvalidSpecException;
/**
- * Get job info
- * @param jobSpec job spec
- * @return job object
- * @throws UnsupportedJobTypeException caused by the unsupported job type
+ * Get job
+ * @param jobId job id
+ * @return object
+ * @throws UnsupportedSubmitterException unsupported submitter
+ * @throws InvalidSpecException invalid spec
*/
- default Job getJob(JobSpec jobSpec) throws UnsupportedJobTypeException {
- // TODO(submarine) should implementing later
- return null;
- }
+ Job getJob(String jobId) throws UnsupportedSubmitterException, NotFoundJobException,
+ InvalidSpecException;
/**
- * Update job info
- * @param jobSpec job spec
- * @return job object
- * @throws UnsupportedJobTypeException caused by the unsupported job type
+ * List jobs
+ * @param status job status, if null will return all jobs
+ * @return job list
+ * @throws UnsupportedSubmitterException unsupported submitter
+ * @throws InvalidSpecException invalid spec
*/
- default Job updateJob(JobSpec jobSpec) throws UnsupportedJobTypeException {
- // TODO(submarine) should implementing later
- return null;
- }
+ List<Job> listJobs(String status) throws UnsupportedSubmitterException, InvalidSpecException;
Review comment:
How about we change listJobs to listJobsByJobStatus. Since we may need functions like listJobsByUser or listJobsByJobType in the future.
----------------------------------------------------------------
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 issue #249: SUBMARINE-455.
Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
tangzhankun commented on issue #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#issuecomment-611301167
@jiwq , Thanks for the great contribution. It looks good to me. +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] jiwq commented on a change in pull request #249:
SUBMARINE-455. Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
jiwq commented on a change in pull request #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#discussion_r404063401
##########
File path: submarine-cloud/hack/output/bin/kubectl
##########
@@ -0,0 +1 @@
+/usr/local/bin/kubectl
Review comment:
Sorry for that, I add submarine-cloud/hack/output/* in .gitignore, but I add these to git. I'll remove 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] yuanzac commented on a change in pull request #249:
SUBMARINE-455. Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
yuanzac commented on a change in pull request #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#discussion_r399095947
##########
File path: submarine-server/server-submitter/submitter-k8s/src/test/resources/pytorch_job_mnist_gloo.json
##########
@@ -1,47 +0,0 @@
-{
- "apiVersion": "kubeflow.org/v1",
Review comment:
We don't need this for tests?
----------------------------------------------------------------
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 #249:
SUBMARINE-455. Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
liuxunorg commented on a change in pull request #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#discussion_r404023376
##########
File path: submarine-test/test-k8s/src/test/java/org/apache/submarine/server/client/HttpClientManager.java
##########
@@ -0,0 +1,74 @@
+/*
+ * 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.client;
+
+import java.io.IOException;
+
+import com.google.common.annotations.VisibleForTesting;
+import org.apache.commons.httpclient.HttpClient;
+import org.apache.commons.httpclient.cookie.CookiePolicy;
+import org.apache.commons.httpclient.methods.DeleteMethod;
+import org.apache.commons.httpclient.methods.GetMethod;
+import org.apache.commons.httpclient.methods.PostMethod;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Only used Integration Testing.
+ */
+@VisibleForTesting
+public class HttpClientManager {
Review comment:
We already have base class `AbstractSubmarineServerTest`, which can help us perform `get`, `post`, `delete` http operations
----------------------------------------------------------------
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 #249:
SUBMARINE-455. Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
liuxunorg commented on a change in pull request #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#discussion_r404015469
##########
File path: conf/submarine-site.xml.template
##########
@@ -155,4 +155,28 @@
<description>Rpc server port</description>
</property>
+ <!-- Submarine Submitters Configuration -->
+ <property>
+ <name>submarine.submitters</name>
+ <value>k8s</value>
+ <description>Submitter list for the server</description>
+ </property>
+ <property>
+ <name>submarine.submitters.k8s.class</name>
Review comment:
This configuration is not required because this `org.apache.submarine.server.submitter.k8s.K8sJobSubmitter` is fixed.
----------------------------------------------------------------
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] jiwq commented on a change in pull request #249:
SUBMARINE-455. Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
jiwq commented on a change in pull request #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#discussion_r404065951
##########
File path: conf/submarine-site.xml.template
##########
@@ -155,4 +155,28 @@
<description>Rpc server port</description>
</property>
+ <!-- Submarine Submitters Configuration -->
+ <property>
+ <name>submarine.submitters</name>
+ <value>k8s</value>
+ <description>Submitter list for the server</description>
+ </property>
+ <property>
+ <name>submarine.submitters.k8s.class</name>
+ <value>org.apache.submarine.server.submitter.k8s.K8sJobSubmitter</value>
+ <description>The entry class for the specified submitter</description>
+ </property>
+ <property>
+ <name>submarine.submitters.k8s.classpath</name>
+ <value>../lib/submitter/k8s/</value>
+ <description>The libs for Kubernetes submitter</description>
+ </property>
+
+ <!-- K8s Configuration -->
+ <property>
+ <name>submarine.k8s.kube.config</name>
Review comment:
Sorry for that, I forgot to remove it. In cluster mode, we don't need this.
----------------------------------------------------------------
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 #249:
SUBMARINE-455. Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
liuxunorg commented on a change in pull request #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#discussion_r404030638
##########
File path: submarine-server/server-core/src/main/java/org/apache/submarine/server/response/JsonResponse.java
##########
@@ -166,7 +182,6 @@ public String toString() {
.registerTypeAdapter(Date.class, safeDateTypeAdapter)
.registerTypeAdapter(JobId.class, new JobIdSerializer())
.registerTypeAdapter(JobId.class, new JobIdDeserializer())
- .serializeNulls()
Review comment:
Why should I delete this line?
----------------------------------------------------------------
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 #249:
SUBMARINE-455. Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
liuxunorg commented on a change in pull request #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#discussion_r405428857
##########
File path: submarine-server/server-core/src/main/java/org/apache/submarine/server/job/JobServiceException.java
##########
@@ -0,0 +1,46 @@
+/*
+ * 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.job;
+
+import javax.ws.rs.core.Response;
+
+import org.apache.submarine.server.api.exception.SubmitterException;
+
+public class JobServiceException extends RuntimeException {
Review comment:
Now the extended `*Exception` makes the system very complicated.
It is sufficient to use `SubmarineRuntimeException` in the commons-utils module.
----------------------------------------------------------------
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 #249:
SUBMARINE-455. Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
liuxunorg commented on a change in pull request #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#discussion_r403973221
##########
File path: submarine-server/server-core/src/main/java/org/apache/submarine/server/response/JsonResponse.java
##########
@@ -48,6 +48,7 @@
private final javax.ws.rs.core.Response.Status status;
private final int code;
private final Boolean success;
+ private final Integer errorCode;
Review comment:
JsonResponse.java already have a `code` field, why do I need to add an `error code` field?
----------------------------------------------------------------
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 #249:
SUBMARINE-455. Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
liuxunorg commented on a change in pull request #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#discussion_r404034109
##########
File path: submarine-cloud/hack/output/bin/kubectl
##########
@@ -0,0 +1 @@
+/usr/local/bin/kubectl
Review comment:
Need delete `submarine-cloud/hack/output/bin/kubectl` file.
kubectl will automatically download.
----------------------------------------------------------------
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 #249:
SUBMARINE-455. Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
tangzhankun commented on a change in pull request #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#discussion_r399740591
##########
File path: submarine-test/test-k8s/src/test/java/org/apache/submarine/rest/JobManagerRestApiIT.java
##########
@@ -60,7 +60,7 @@ public void testCreateJob() throws Exception {
Gson gson = new Gson();
JsonResponse jsonResponse = gson.fromJson(responseBodyAsString, JsonResponse.class);
- assertEquals("Response code should be 202 ",
- Response.Status.ACCEPTED.getStatusCode(), jsonResponse.getCode());
+ assertEquals("Response code should be 406 ",
Review comment:
I'm kind of confusing with this one and the JobManagerRestApiTest (AbstractSubmarineServerTest). Both the two seem to start submarine-server locally and do REST queries.
If we'd prefer this JobManagerRestApiIT as integration and end-to-end test, then we should figure out a way to mock the submarine server instead of start a real submarine server in the JobManagerRestApiTest. Thoughts? @jiwq , @liuxunorg , @yuanzac
----------------------------------------------------------------
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 a change in pull request #249:
SUBMARINE-455. Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
yuanzac commented on a change in pull request #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#discussion_r404078907
##########
File path: submarine-server/server-core/src/test/java/org/apache/submarine/server/submitter/mock/MockSubmitter.java
##########
@@ -0,0 +1,89 @@
+/*
+ * 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.submitter.mock;
+
+import java.util.Date;
+import java.util.concurrent.ConcurrentHashMap;
+
+import org.apache.submarine.commons.utils.SubmarineConfiguration;
+import org.apache.submarine.server.api.job.JobSubmitter;
+import org.apache.submarine.server.api.exception.InvalidSpecException;
+import org.apache.submarine.server.api.job.Job;
+import org.apache.submarine.server.api.spec.JobSpec;
+
+public class MockSubmitter implements JobSubmitter {
+ // key is job name, value is job object
+ private ConcurrentHashMap<String, Job> cachedJob = new ConcurrentHashMap<>();
+
+ @Override
+ public void initialize(SubmarineConfiguration conf) {
+ System.out.println("mock");
+ }
+
+ @Override
+ public String getSubmitterType() {
+ return "mock";
+ }
+
+ @Override
+ public Job createJob(JobSpec jobSpec) throws InvalidSpecException {
+ String jobName = jobSpec.getName();
+ if (cachedJob.get(jobName) != null) {
+ return null;
+ }
+ Job job = new Job();
+ job.setSpec(jobSpec);
+ job.setName(jobName);
+ job.setStatus(Job.Status.STATUS_ACCEPTED.getValue());
+ job.setAcceptedTime(new Date().toString());
+ cachedJob.put(jobName, job);
+ return job;
+ }
+
+ @Override
+ public Job findJob(JobSpec jobSpec) throws InvalidSpecException {
+ String jobName = jobSpec.getName();
+ return cachedJob.get(jobName);
+ }
+
+ @Override
+ public Job patchJob(JobSpec jobSpec) throws InvalidSpecException {
+ String jobName = jobSpec.getName();
+ Job job = cachedJob.get(jobName);
+ if (job == null) {
+ return null;
+ }
+ job.setStatus(Job.Status.STATUS_ACCEPTED.getValue());
+ job.setCreatedTime(new Date().toString());
Review comment:
Why do we need to update create time when patchJob is called?
----------------------------------------------------------------
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 #249:
SUBMARINE-455. Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
tangzhankun commented on a change in pull request #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#discussion_r399739696
##########
File path: submarine-server/server-core/src/test/java/org/apache/submarine/server/rest/JobManagerRestApiTest.java
##########
@@ -137,16 +134,12 @@ public void testGetJobById() throws IOException {
.create();
JsonResponse<Job> jsonResponse = gson.fromJson(responseBodyAsString,
new TypeToken<JsonResponse<Job>>(){}.getType());
- assertEquals("Response code should be 200 ",
- Response.Status.OK.getStatusCode(), jsonResponse.getCode());
- assertEquals("Job id should be " + jobId, jobId,
- jsonResponse.getResult().getJobId().toString());
+ assertEquals(Response.Status.NOT_FOUND.getStatusCode(), jsonResponse.getCode());
Review comment:
This probably needs to mock the JobManager internals job list beforehand or post jobs specs to ensure the JobManager store it before the query.
@liuxunorg any better idea or help on how to mock the submarine server? The AbstractSubmarineServerTest seems is starting up a real submarine server. To me it seems not easy to mock the submarine server's internals easily.
----------------------------------------------------------------
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] jiwq commented on a change in pull request #249:
SUBMARINE-455. Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
jiwq commented on a change in pull request #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#discussion_r404059269
##########
File path: conf/submarine-site.xml.template
##########
@@ -155,4 +155,28 @@
<description>Rpc server port</description>
</property>
+ <!-- Submarine Submitters Configuration -->
+ <property>
+ <name>submarine.submitters</name>
+ <value>k8s</value>
+ <description>Submitter list for the server</description>
+ </property>
+ <property>
+ <name>submarine.submitters.k8s.class</name>
Review comment:
As we discussed offline, I got it will be fixed in the next. At now we should config 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] yuanzac commented on a change in pull request #249:
SUBMARINE-455. Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
yuanzac commented on a change in pull request #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#discussion_r399090024
##########
File path: submarine-server/server-submitter/submitter-k8s/src/test/java/org/apache/submarine/server/submitter/k8s/K8SJobSubmitterTest.java
##########
@@ -62,21 +51,10 @@
* Local: docker run -it --privileged -p 8443:8443 -p 10080:10080 bsycorp/kind:latest-1.15
* Travis: See '.travis.yml'
*/
-public class K8SJobSubmitterTest {
- private final String tfJobName = "mnist";
- private final String pytorchJobName = "pytorch-dist-mnist-gloo";
-
- // The spec files in test/resources
- private final String tfJobSpecFile = "/tf_job_mnist.json";
- private final String tfJobReqFile = "/tf_mnist_req.json";
- private final String pytorchJobSpecFile = "/pytorch_job_mnist_gloo.json";
- private final String pytorchJobReqFile = "/pytorch_job_req.json";
+public class K8SJobSubmitterTest extends SpecBuilder {
Review comment:
It looks like we don't have tests for patchJob and listJob. We need to add them.
----------------------------------------------------------------
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 #249:
SUBMARINE-455. Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
liuxunorg commented on a change in pull request #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#discussion_r405429202
##########
File path: submarine-server/server-api/src/main/java/org/apache/submarine/server/api/exception/SubmitterException.java
##########
@@ -0,0 +1,104 @@
+/*
+ * 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.exception;
+
+import org.apache.submarine.commons.utils.exception.SubmarineRuntimeException;
+
+/**
+ * SubmitterException is caused by internal or upstream server running error.
+ */
+public class SubmitterException extends SubmarineRuntimeException {
+ private final int code;
Review comment:
Now the extended `*Exception` makes the system very complicated.
It is sufficient to use `SubmarineRuntimeException` in the commons-utils module.
----------------------------------------------------------------
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 #249:
SUBMARINE-455. Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
tangzhankun commented on a change in pull request #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#discussion_r405387513
##########
File path: submarine-server/server-core/src/main/java/org/apache/submarine/server/job/JobManager.java
##########
@@ -0,0 +1,214 @@
+/*
+ * 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.job;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
+import java.util.concurrent.atomic.AtomicInteger;
+
+import javax.ws.rs.core.Response.Status;
+
+import org.apache.submarine.commons.utils.SubmarineConfiguration;
+import org.apache.submarine.server.SubmarineServer;
+import org.apache.submarine.server.SubmitterManager;
+import org.apache.submarine.server.api.exception.SubmitterException;
+import org.apache.submarine.server.api.job.JobSubmitter;
+import org.apache.submarine.server.api.job.Job;
+import org.apache.submarine.server.api.job.JobId;
+import org.apache.submarine.server.api.spec.JobSpec;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * It responsible for manage the job (CRUD) and cached the job.
Review comment:
How about "It's responsible for managing the jobs CRUD and cache them"?
----------------------------------------------------------------
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] jiwq commented on a change in pull request #249:
SUBMARINE-455. Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
jiwq commented on a change in pull request #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#discussion_r404059162
##########
File path: conf/submarine-site.xml.template
##########
@@ -155,4 +155,28 @@
<description>Rpc server port</description>
</property>
+ <!-- Submarine Submitters Configuration -->
+ <property>
+ <name>submarine.submitters</name>
+ <value>k8s</value>
+ <description>Submitter list for the server</description>
+ </property>
+ <property>
+ <name>submarine.submitters.k8s.class</name>
+ <value>org.apache.submarine.server.submitter.k8s.K8sJobSubmitter</value>
+ <description>The entry class for the specified submitter</description>
+ </property>
+ <property>
+ <name>submarine.submitters.k8s.classpath</name>
Review comment:
As we discussed offline, I got it will be fixed in the next. At now we should config 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] liuxunorg commented on a change in pull request #249:
SUBMARINE-455. Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
liuxunorg commented on a change in pull request #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#discussion_r404013362
##########
File path: conf/submarine-site.xml.template
##########
@@ -155,4 +155,28 @@
<description>Rpc server port</description>
</property>
+ <!-- Submarine Submitters Configuration -->
+ <property>
+ <name>submarine.submitters</name>
+ <value>k8s</value>
+ <description>Submitter list for the server</description>
+ </property>
+ <property>
+ <name>submarine.submitters.k8s.class</name>
+ <value>org.apache.submarine.server.submitter.k8s.K8sJobSubmitter</value>
+ <description>The entry class for the specified submitter</description>
+ </property>
+ <property>
+ <name>submarine.submitters.k8s.classpath</name>
Review comment:
This configuration is not required because this path(`../lib/submitter/k8s/`) is fixed.
----------------------------------------------------------------
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 #249:
SUBMARINE-455. Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
liuxunorg commented on a change in pull request #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#discussion_r404017958
##########
File path: submarine-test/test-k8s/src/test/resources/pytorch/pt-mnist-patch-req.yaml
##########
@@ -0,0 +1,21 @@
+---
+name: mnist
+librarySpec:
+ name: pytorch
+ version: 2.1.0
+ image: apache/submarine:pytorch-dist-mnist-1.0
+ cmd: python /var/mnist.py --backend gloo
+ envVars:
+ ENV_1: ENV1
+submitterSpec:
+ type: k8s
Review comment:
We don't need
```type: "k8s"```
The submission to `k8s` or `yarn` is determined by submarine, not by the user.
----------------------------------------------------------------
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 #249:
SUBMARINE-455. Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
liuxunorg commented on a change in pull request #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#discussion_r404018011
##########
File path: submarine-test/test-k8s/src/test/resources/pytorch/pt-mnist-patch-req.json
##########
@@ -0,0 +1,28 @@
+{
+ "name": "mnist",
+ "librarySpec": {
+ "name": "pytorch",
+ "version": "2.1.0",
+ "image": "apache/submarine:pytorch-dist-mnist-1.0",
+ "cmd": "python /var/mnist.py --backend gloo",
+ "envVars": {
+ "ENV_1": "ENV1"
+ }
+ },
+ "submitterSpec": {
+ "type": "k8s",
Review comment:
We don't need
```type: "k8s"```
The submission to `k8s` or `yarn` is determined by submarine, not by the user.
----------------------------------------------------------------
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 #249:
SUBMARINE-455. Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
tangzhankun commented on a change in pull request #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#discussion_r399739124
##########
File path: submarine-server/server-core/src/test/java/org/apache/submarine/server/rest/JobManagerRestApiTest.java
##########
@@ -90,8 +90,7 @@ public void testCreateJobWhenJsonInputIsCorrectThenResponseCodeAccepted() throws
Gson gson = new Gson();
JsonResponse jsonResponse = gson.fromJson(responseBodyAsString, JsonResponse.class);
- assertEquals("Response code should be 202 ",
- Response.Status.ACCEPTED.getStatusCode(), jsonResponse.getCode());
+ assertEquals(Response.Status.NOT_ACCEPTABLE.getStatusCode(), jsonResponse.getCode());
Review comment:
why it response not acceptable?
----------------------------------------------------------------
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 a change in pull request #249:
SUBMARINE-455. Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
yuanzac commented on a change in pull request #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#discussion_r399096084
##########
File path: submarine-server/server-submitter/submitter-k8s/src/test/resources/tf_job_mnist.json
##########
@@ -1,48 +0,0 @@
-{
Review comment:
we don't need this for 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] liuxunorg commented on issue #249: SUBMARINE-455.
Support CRUD job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
liuxunorg commented on issue #249: SUBMARINE-455. Support CRUD job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#issuecomment-611610587
@jiwq , Thanks for the great contribution!
Will merge if no more 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] [submarine] yuanzac commented on a change in pull request #249:
SUBMARINE-455. Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
yuanzac commented on a change in pull request #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#discussion_r404083876
##########
File path: submarine-server/server-submitter/submitter-k8s/src/main/java/org/apache/submarine/server/submitter/k8s/mapper/MLJobMapper.java
##########
@@ -0,0 +1,97 @@
+/*
+ * 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.submitter.k8s.mapper;
+
+import java.util.List;
+
+import io.kubernetes.client.models.V1DeleteOptions;
+import io.kubernetes.client.models.V1DeleteOptionsBuilder;
+import io.kubernetes.client.models.V1JobCondition;
+import io.kubernetes.client.models.V1JobStatus;
+import io.kubernetes.client.models.V1Status;
+import io.kubernetes.client.models.V1StatusDetails;
+import org.apache.submarine.server.api.job.Job;
+import org.apache.submarine.server.submitter.k8s.model.MLJob;
+import org.joda.time.DateTime;
+
+/**
+ * Mappings between Java bean types.
+ * Such as MLJob to Job, V1Status to Job and others.
+ */
+public class MLJobMapper {
+ public static Job toJob(MLJob mlJob) {
+ Job job = new Job();
+ job.setUid(mlJob.getMetadata().getUid());
+ job.setName(mlJob.getMetadata().getName());
+
+ DateTime dateTime = mlJob.getMetadata().getCreationTimestamp();
+ if (dateTime != null) {
+ job.setAcceptedTime(dateTime.toString());
+ job.setStatus(Job.Status.STATUS_ACCEPTED.getValue());
+ }
+
+ V1JobStatus status = mlJob.getStatus();
+ if (status != null) {
+ dateTime = status.getStartTime();
+ if (dateTime != null) {
+ job.setCreatedTime(dateTime.toString());
+ job.setStatus(Job.Status.STATUS_CREATED.getValue());
+ }
+
+ List<V1JobCondition> conditions = status.getConditions();
+ if (conditions != null && conditions.size() > 1) {
+ job.setStatus(Job.Status.STATUS_RUNNING.getValue());
+ for (V1JobCondition condition : conditions) {
+ if (Boolean.parseBoolean(condition.getStatus())
+ && condition.getType().toLowerCase().equals(
+ "running")) {
+ dateTime = condition.getLastTransitionTime();
+ job.setRunningTime(dateTime.toString());
+ break;
+ }
+ }
+ }
+
+ dateTime = status.getCompletionTime();
+ if (dateTime != null) {
+ job.setFinishedTime(dateTime.toString());
+ job.setStatus(Job.Status.STATUS_SUCCEEDED.getValue());
+ }
+ }
+ return job;
+ }
+
+ public static Job toJob(V1Status status) {
+ Job job = new Job();
+ V1StatusDetails details = status.getDetails();
+ if (details != null) {
+ job.setUid(details.getUid());
+ job.setName(details.getName());
+ }
+ if (status.getStatus().toLowerCase().equals("success")) {
+ job.setStatus(Job.Status.STATUS_DELETED.getValue());
Review comment:
The job status is set to STATUS_DELETED not STATUS_SUCCEEDED?
----------------------------------------------------------------
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] jiwq commented on a change in pull request #249:
SUBMARINE-455. Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
jiwq commented on a change in pull request #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#discussion_r404063289
##########
File path: submarine-cloud/hack/output/bin/kind
##########
@@ -0,0 +1 @@
+/usr/local/bin/kind
Review comment:
Sorry for that, I add `submarine-cloud/hack/output/*` in `.gitignore`, but I add these to git. I'll remove 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] liuxunorg commented on issue #249: SUBMARINE-455.
Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
liuxunorg commented on issue #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#issuecomment-610266567
@jiwq Please change file name `dev-support/k8s/deploy.sh` to `dev-support/k8s/deploy-kubeflow-operator.sh `.
----------------------------------------------------------------
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] jiwq edited a comment on issue #249: SUBMARINE-455.
Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
jiwq edited a comment on issue #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#issuecomment-604933388
Thanks @liuxunorg and @yuanzac reviewing. I will complete the unit test as soon later.
> Please change var name `jobIdString` to `jobId`.
Good catch, considering the future develop maybe we can use the `id` to replace the `jobIdString`. Because `id` looks simple and cleaner, and its meaning is more intuitive.
> Should we need deleted the tf_job_mnist.json and pytorch_job_mnist_gloo.json files?
Yes, we used the submarine job spec file replace it. By the way the `k8s submitter` only supports the `JobSpec` as the input param. So we don't need this files no longer.
> Since K8sJobSubmitter is public, we don't need to add "@VisibleForTesting"
Good catch. Due to the `SubmitterManager` only supports non params constructor, so I think we should add the annotation to indicate the method only used in the test code. Thoughts?
> We'd better change findJob() to getJob(), as jobhandler uses getJob().
In my opinion, used the `find` prefix is better that `get`, because we often use the `get` prefix in the getter method. Any thoughts?
----------------------------------------------------------------
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] jiwq commented on issue #249: SUBMARINE-455. Support
find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
jiwq commented on issue #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#issuecomment-604933388
Thanks @liuxunorg and @yuanzac reviewing. I will complete the unit test as soon later.
> Should we need deleted the tf_job_mnist.json and pytorch_job_mnist_gloo.json files?
Yes, we used the submarine job spec file replace it. By the way the `k8s submitter` only supports the `JobSpec` as the input param. So we don't need this files no longer.
> Since K8sJobSubmitter is public, we don't need to add "@VisibleForTesting"
Good catch. Due to the `SubmitterManager` only supports non params constructor, so I think we should add the annotation to indicate the method only used in the test code. Thoughts?
> We'd better change findJob() to getJob(), as jobhandler uses getJob().
In my opinion, used the `find` prefix is better that `get`, because we often use the `get` prefix in the getter method. Any thoughts?
----------------------------------------------------------------
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 #249:
SUBMARINE-455. Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
liuxunorg commented on a change in pull request #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#discussion_r398277370
##########
File path: submarine-server/server-submitter/submitter-k8s/src/main/java/org/apache/submarine/server/submitter/k8s/K8sJobSubmitter.java
##########
@@ -101,98 +101,78 @@ public String getSubmitterType() {
}
@Override
- public Job submitJob(JobSpec jobSpec)
- throws InvalidSpecException {
+ public Job createJob(JobSpec jobSpec) throws InvalidSpecException {
Job job = null;
-
- boolean success = createJob(JobSpecParser.parseJob(jobSpec));
- if (success) {
- job = new Job();
- job.setName(jobSpec.getName());
- } else {
- LOG.error("Failed to create job." + jobSpec.toString());
+ try {
+ MLJob mlJob = JobSpecParser.parseJob(jobSpec);
+ Object object = api.createNamespacedCustomObject(mlJob.getGroup(), mlJob.getVersion(),
+ mlJob.getMetadata().getNamespace(), mlJob.getPlural(), mlJob, "true");
+ job = parseResponseObject(object, ParseOp.PARSE_OP_RESULT);
+ } catch (ApiException e) {
+ LOG.error("Failed to create job. " + e.getMessage(), e);
}
return job;
}
- @VisibleForTesting
- boolean createJob(MLJob job) {
+ @Override
+ public Job findJob(JobSpec jobSpec) throws InvalidSpecException {
+ Job job = null;
try {
- api.createNamespacedCustomObject(job.getGroup(), job.getVersion(),
- job.getMetadata().getNamespace(), job.getPlural(),
- job, "true");
+ MLJob mlJob = JobSpecParser.parseJob(jobSpec);
+ Object object = api.getNamespacedCustomObject(mlJob.getGroup(), mlJob.getVersion(),
+ mlJob.getMetadata().getNamespace(), mlJob.getPlural(), mlJob.getMetadata().getName());
+ job = parseResponseObject(object, ParseOp.PARSE_OP_RESULT);
} catch (ApiException e) {
- LOG.error("Failed to create job. " + e.getMessage(), e);
- return false;
+ LOG.warn("Failed to get job. " + e.getMessage(), e);
}
- return true;
+ return job;
}
- @VisibleForTesting
- CustomResourceJob createCustomJob(K8sJobRequest request) {
+ @Override
+ public Job patchJob(JobSpec jobSpec) throws InvalidSpecException {
+ Job job = null;
try {
- K8sJobRequest.Path path = request.getPath();
- Object o = api.createNamespacedCustomObject(path.getGroup(),
- path.getApiVersion(), path.getNamespace(), path.getPlural(),
- request.getBody(), "true");
- Gson gson = new Gson();
- return gson.fromJson(gson.toJson(o), CustomResourceJob.class);
- } catch (ApiException ae) {
- LOG.error("Exceptions when creating CRD job: " + ae.getMessage(), ae);
+ MLJob mlJob = JobSpecParser.parseJob(jobSpec);
+ Object object = api.patchNamespacedCustomObject(mlJob.getGroup(), mlJob.getVersion(),
Review comment:
I think, you should first determine whether the job name does not exist, and return an accurate error message if it does not exist.
----------------------------------------------------------------
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 #249:
SUBMARINE-455. Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
tangzhankun commented on a change in pull request #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#discussion_r399740591
##########
File path: submarine-test/test-k8s/src/test/java/org/apache/submarine/rest/JobManagerRestApiIT.java
##########
@@ -60,7 +60,7 @@ public void testCreateJob() throws Exception {
Gson gson = new Gson();
JsonResponse jsonResponse = gson.fromJson(responseBodyAsString, JsonResponse.class);
- assertEquals("Response code should be 202 ",
- Response.Status.ACCEPTED.getStatusCode(), jsonResponse.getCode());
+ assertEquals("Response code should be 406 ",
Review comment:
I'm kind of confusing with this one and the JobManagerRestApiTest (AbstractSubmarineServerTest). Both the two seem to start submarine-server locally and do REST queries.
If we'd prefer this JobManagerRestApiIT as integration and end-to-end test, then we should figure out a way to mock the submarine server instead of start a real submarine server in the JobManagerRestApiTest. Thoughts? @jiwq , @liuxunorg , @yuanzac
----------------------------------------------------------------
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 #249:
SUBMARINE-455. Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
liuxunorg commented on a change in pull request #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#discussion_r404017666
##########
File path: submarine-test/test-k8s/src/test/resources/tensorflow/tf-mnist-patch-req.yaml
##########
@@ -0,0 +1,19 @@
+name: "mnist"
+librarySpec:
+ name: "TensorFlow"
+ version: "2.1.0"
+ image: "gcr.io/kubeflow-ci/tf-mnist-with-summaries:1.0"
+ cmd: "python /var/tf_mnist/mnist_with_summaries.py --log_dir=/train/log --learning_rate=0.01 --batch_size=150"
+ envVars:
+ ENV_1: "ENV1"
+submitterSpec:
+ type: "k8s"
Review comment:
We don't need
```type: "k8s"```
The submission to `k8s` or `yarn` is determined by submarine, not by the user.
----------------------------------------------------------------
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 #249:
SUBMARINE-455. Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
liuxunorg commented on a change in pull request #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#discussion_r398277186
##########
File path: submarine-server/server-submitter/submitter-k8s/src/main/java/org/apache/submarine/server/submitter/k8s/K8sJobSubmitter.java
##########
@@ -101,98 +101,78 @@ public String getSubmitterType() {
}
@Override
- public Job submitJob(JobSpec jobSpec)
- throws InvalidSpecException {
+ public Job createJob(JobSpec jobSpec) throws InvalidSpecException {
Job job = null;
-
- boolean success = createJob(JobSpecParser.parseJob(jobSpec));
- if (success) {
- job = new Job();
- job.setName(jobSpec.getName());
- } else {
- LOG.error("Failed to create job." + jobSpec.toString());
+ try {
+ MLJob mlJob = JobSpecParser.parseJob(jobSpec);
Review comment:
I think, you should first determine whether the job name already exists, and return an accurate error message if it exists.
----------------------------------------------------------------
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 #249:
SUBMARINE-455. Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
tangzhankun commented on a change in pull request #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#discussion_r399737655
##########
File path: submarine-server/server-core/src/main/java/org/apache/submarine/server/rest/JobManagerRestApi.java
##########
@@ -59,24 +61,24 @@ public Response ping() {
/**
* Returns the contents of {@link Job} that submitted by user.
- * @param jobSpec job spec
+ * @param spec job spec
* @return the contents of job
*/
@POST
@Consumes({RestConstants.MEDIA_TYPE_YAML, MediaType.APPLICATION_JSON})
- public Response submitJob(JobSpec jobSpec) {
- if (!jobSpec.validate()) {
- return new JsonResponse.Builder<String>(Response.Status.ACCEPTED)
- .success(false).result("Invalid params.").build();
- }
-
+ public Response createJob(JobSpec spec) {
try {
- Job job = JobManager.getInstance().submitJob(jobSpec);
- return new JsonResponse.Builder<Job>(Response.Status.OK)
- .success(true).result(job).build();
- } catch (UnsupportedJobTypeException e) {
- return new JsonResponse.Builder<String>(Response.Status.ACCEPTED)
- .success(false).result(e.getMessage()).build();
+ Job job = JobManager.getInstance().createJob(spec);
Review comment:
We should check if the job exists and return the proper response. This is important. Otherwise, a same k8s job spec with the same job name could be submitted but fail quietly without any hint to the user.
----------------------------------------------------------------
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 a change in pull request #249:
SUBMARINE-455. Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
yuanzac commented on a change in pull request #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#discussion_r399057552
##########
File path: submarine-server/server-api/src/main/java/org/apache/submarine/server/api/JobSubmitter.java
##########
@@ -38,4 +41,35 @@
*/
String getSubmitterType();
+ /**
+ * Create job with job spec
+ * @param jobSpec job spec
+ * @return object
+ * @throws InvalidSpecException invalid spec
+ */
+ Job createJob(JobSpec jobSpec) throws InvalidSpecException;
+
+ /**
+ * Find job by job spec
+ * @param jobSpec job spec
+ * @return object
+ * @throws InvalidSpecException invalid spec
+ */
+ Job findJob(JobSpec jobSpec) throws InvalidSpecException;
Review comment:
We'd better change findJob() to getJob(), as jobhandler uses getJob().
----------------------------------------------------------------
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 #249:
SUBMARINE-455. Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
liuxunorg commented on a change in pull request #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#discussion_r398272920
##########
File path: submarine-server/server-core/src/main/java/org/apache/submarine/server/JobManager.java
##########
@@ -69,36 +70,93 @@ public static JobManager getInstance() {
private JobManager(SubmitterManager submitterManager) {
this.submitterManager = submitterManager;
- this.executorService = Executors.newFixedThreadPool(50);
}
@Override
- public Job submitJob(JobSpec spec) throws UnsupportedJobTypeException {
- if (!spec.validate()) {
- return null;
+ public Job createJob(JobSpec spec) throws UnsupportedSubmitterException, InvalidSpecException {
+ Job job = getSubmitter(spec.getSubmitterSpec().getType()).createJob(spec);
+ if (job != null) {
+ JobId jobId = generateJobId();
+ job.setJobId(jobId);
+ job.setSpec(spec);
+ cachedJobSpecs.putIfAbsent(jobId.toString(), spec);
}
+ return job;
+ }
- JobSubmitter submitter = submitterManager.getSubmitterByType(
- spec.getSubmitterSpec().getType());
- if (submitter == null) {
- throw new UnsupportedJobTypeException();
+ private JobId generateJobId() {
+ return JobId.newInstance(SubmarineServer.getServerTimeStamp(), jobCounter.incrementAndGet());
+ }
+
+ @Override
+ public Job getJob(String jobIdString) throws UnsupportedSubmitterException,
+ NotFoundJobException, InvalidSpecException {
+ JobId jobId = JobId.fromString(jobIdString);
+ if (jobId == null) {
+ throw new NotFoundJobException();
+ }
+ JobSpec spec = cachedJobSpecs.get(jobId.toString());
+ if (spec == null) {
+ throw new NotFoundJobException();
}
+ Job job = getSubmitter(spec.getSubmitterSpec().getType()).findJob(spec);
+ return fillJob(job, jobId, spec);
+ }
- Job job = new Job();
- job.setJobId(generateJobId());
- executorService.submit(() -> {
- try {
- jobs.putIfAbsent(job.getJobId(), submitter.submitJob(spec));
- } catch (UnsupportedJobTypeException e) {
- LOG.error(e.getMessage(), e);
- } catch (InvalidSpecException e) {
- LOG.error("Invalid job spec: " + spec + ", " + e.getMessage());
+ @Override
+ public List<Job> listJobs(String status) throws UnsupportedSubmitterException,
+ InvalidSpecException {
+ List<Job> jobList = new ArrayList<>();
+ for (Map.Entry<String, JobSpec> entry : cachedJobSpecs.entrySet()) {
+ Job job = getSubmitter(entry.getValue().getSubmitterSpec().getType())
+ .findJob(entry.getValue());
+ if (status == null || status.toLowerCase().equals(job.getStatus().toLowerCase())) {
+ jobList.add(fillJob(job, JobId.fromString(entry.getKey()), entry.getValue()));
}
- });
- return job;
+ }
+ return jobList;
}
- private JobId generateJobId() {
- return JobId.newInstance(SubmarineServer.getServerTimeStamp(), jobCounter.incrementAndGet());
+ @Override
+ public Job patchJob(String jobIdString, JobSpec spec) throws UnsupportedSubmitterException,
Review comment:
Please change var name `jobIdString` to `jobId`
The same below
----------------------------------------------------------------
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 #249:
SUBMARINE-455. Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
liuxunorg commented on a change in pull request #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#discussion_r404032514
##########
File path: submarine-server/server-api/src/main/java/org/apache/submarine/server/api/exception/ServiceException.java
##########
@@ -0,0 +1,73 @@
+/*
+ * 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.exception;
+
+/**
+ * The class {@code ServiceException} and its subclasses indicates that a reasonable service
+ * exception. The service code can be divided to system and service level. System status code
+ * between 10000 and 19999, service status code between 20000 and 29999.
+ *
+ * Such as:
+ * System: 1xxxxx (1 indicate is system level)
+ * Service: 2xxxxx (2 indicate is service level)
+ * Job Service: 201xxx (01 indicate job service component)
Review comment:
Need to add documentation to explain.
----------------------------------------------------------------
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 #249:
SUBMARINE-455. Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
liuxunorg commented on a change in pull request #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#discussion_r404039420
##########
File path: submarine-cloud/hack/deploy-submarine.sh
##########
@@ -57,7 +67,10 @@ function install_submarine() {
if $KUBECTL_BIN get configmap --namespace default | grep submarine-config >/dev/null ; then
$KUBECTL_BIN delete configmap --namespace default submarine-config
fi
- $KUBECTL_BIN create configmap --namespace default submarine-config --from-file=${ROOT}/hack/conf/submarine-site.xml --from-file=${ROOT}/hack/conf/log4j.properties
+ $KUBECTL_BIN create configmap --namespace default submarine-config \
+ --from-file=${ROOT}/hack/conf/submarine-site.xml \
+ --from-file=${ROOT}/hack/conf/log4j.properties \
+ --from-file=${ROOT}/hack/conf/k8s/config
Review comment:
manifests/submarine-cluster/submarine-server.yaml:80 already have
```
volumes:
- name: submarine-configmap
configMap:
name: submarine-config
items:
- key: submarine-site.xml
path: submarine-site.xml
- key: log4j.properties
path: log4j.properties
```
----------------------------------------------------------------
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] jiwq commented on a change in pull request #249:
SUBMARINE-455. Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
jiwq commented on a change in pull request #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#discussion_r404058292
##########
File path: submarine-server/server-core/src/main/java/org/apache/submarine/server/response/JsonResponse.java
##########
@@ -48,6 +48,7 @@
private final javax.ws.rs.core.Response.Status status;
private final int code;
private final Boolean success;
+ private final Integer errorCode;
Review comment:
The `code` is the http response code, the `errorCode` meaning the business error.
----------------------------------------------------------------
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] jiwq commented on a change in pull request #249:
SUBMARINE-455. Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
jiwq commented on a change in pull request #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#discussion_r399176882
##########
File path: submarine-server/server-api/src/main/java/org/apache/submarine/server/api/JobHandler.java
##########
@@ -19,53 +19,64 @@
package org.apache.submarine.server.api;
+import java.util.List;
+
import org.apache.submarine.server.api.exception.InvalidSpecException;
-import org.apache.submarine.server.api.exception.UnsupportedJobTypeException;
+import org.apache.submarine.server.api.exception.NotFoundJobException;
+import org.apache.submarine.server.api.exception.UnsupportedSubmitterException;
import org.apache.submarine.server.api.job.Job;
import org.apache.submarine.server.api.spec.JobSpec;
/**
- * Handle the job's operate (CRUD)
+ * Handle the job's operation (CRUD)
*/
public interface JobHandler {
/**
- * Submit job
+ * Create job
* @param jobSpec job spec
- * @return job object
- * @throws UnsupportedJobTypeException caused by the unsupported job type
+ * @return object
+ * @throws UnsupportedSubmitterException unsupported submitter
+ * @throws InvalidSpecException invalid spec
*/
- Job submitJob(JobSpec jobSpec) throws UnsupportedJobTypeException, InvalidSpecException;
+ Job createJob(JobSpec jobSpec) throws UnsupportedSubmitterException, InvalidSpecException;
/**
- * Get job info
- * @param jobSpec job spec
- * @return job object
- * @throws UnsupportedJobTypeException caused by the unsupported job type
+ * Get job
+ * @param jobId job id
+ * @return object
+ * @throws UnsupportedSubmitterException unsupported submitter
+ * @throws InvalidSpecException invalid spec
*/
- default Job getJob(JobSpec jobSpec) throws UnsupportedJobTypeException {
- // TODO(submarine) should implementing later
- return null;
- }
+ Job getJob(String jobId) throws UnsupportedSubmitterException, NotFoundJobException,
+ InvalidSpecException;
/**
- * Update job info
- * @param jobSpec job spec
- * @return job object
- * @throws UnsupportedJobTypeException caused by the unsupported job type
+ * List jobs
+ * @param status job status, if null will return all jobs
+ * @return job list
+ * @throws UnsupportedSubmitterException unsupported submitter
+ * @throws InvalidSpecException invalid spec
*/
- default Job updateJob(JobSpec jobSpec) throws UnsupportedJobTypeException {
- // TODO(submarine) should implementing later
- return null;
- }
+ List<Job> listJobs(String status) throws UnsupportedSubmitterException, InvalidSpecException;
Review comment:
Good idea, I will fix 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] liuxunorg commented on issue #249: SUBMARINE-455.
Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
liuxunorg commented on issue #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#issuecomment-609763993
@jiwq I think that `org.apache.submarine.server.submitter.mock.MockSubmitter` is meaningless because:
1. We need to test in k8s-test.
2. MockSubmitter cannot be submitted to yarn or k8s cluster for testing
3. MockSubmitter also introduces many job json and new class files, which will increase the maintenance workload of the system.
So, I decided to simulate the relevant code of the test, need to delete.
----------------------------------------------------------------
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] jiwq commented on a change in pull request #249:
SUBMARINE-455. Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
jiwq commented on a change in pull request #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#discussion_r404059162
##########
File path: conf/submarine-site.xml.template
##########
@@ -155,4 +155,28 @@
<description>Rpc server port</description>
</property>
+ <!-- Submarine Submitters Configuration -->
+ <property>
+ <name>submarine.submitters</name>
+ <value>k8s</value>
+ <description>Submitter list for the server</description>
+ </property>
+ <property>
+ <name>submarine.submitters.k8s.class</name>
+ <value>org.apache.submarine.server.submitter.k8s.K8sJobSubmitter</value>
+ <description>The entry class for the specified submitter</description>
+ </property>
+ <property>
+ <name>submarine.submitters.k8s.classpath</name>
Review comment:
As we discussed offline, I got it will be fixed in the next. At now we should config 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] liuxunorg commented on a change in pull request #249:
SUBMARINE-455. Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
liuxunorg commented on a change in pull request #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#discussion_r404017639
##########
File path: submarine-test/test-k8s/src/test/resources/tensorflow/tf-mnist-req.json
##########
@@ -11,18 +11,18 @@
},
"submitterSpec": {
"type": "k8s",
Review comment:
We don't need
```type: "k8s"```
The submission to `k8s` or `yarn` is determined by submarine, not by the user.
----------------------------------------------------------------
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] jiwq commented on issue #249: SUBMARINE-455. Support
CRUD job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
jiwq commented on issue #249: SUBMARINE-455. Support CRUD job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#issuecomment-611628040
@liuxunorg @tangzhankun @yuanzac Thank you for being patient and helping me improve.
----------------------------------------------------------------
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 #249:
SUBMARINE-455. Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
tangzhankun commented on a change in pull request #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#discussion_r399738169
##########
File path: submarine-server/server-core/src/main/java/org/apache/submarine/server/rest/JobManagerRestApi.java
##########
@@ -99,11 +109,43 @@ public Response listJob() {
@GET
@Path("{" + RestConstants.JOB_ID + "}")
public Response getJob(@PathParam(RestConstants.JOB_ID) String id) {
- // TODO(jiwq): Hook JobManager when 0.4.0 released
- Job job = new Job();
- job.setJobId(JobId.fromString(id));
- return new JsonResponse.Builder<Job>(Response.Status.OK)
- .success(true).result(job).build();
+ try {
+ Job job = JobManager.getInstance().getJob(id);
+ if (job != null) {
+ return new JsonResponse.Builder<Job>(Response.Status.OK).success(true)
+ .result(job).build();
+ }
+ return new JsonResponse.Builder<String>(Response.Status.NOT_FOUND).success(true).build();
+ } catch (UnsupportedSubmitterException | InvalidSpecException ue) {
+ return new JsonResponse.Builder<String>(Response.Status.NOT_ACCEPTABLE)
+ .message(ue.getMessage()).build();
+ } catch (NotFoundJobException ne) {
+ return new JsonResponse.Builder<String>(Response.Status.NOT_FOUND).success(true).build();
+ } catch (Exception e) {
+ return new JsonResponse.Builder<String>(Response.Status.INTERNAL_SERVER_ERROR)
+ .message(e.getMessage()).build();
+ }
+ }
+
+ @PATCH
+ @Path("{" + RestConstants.JOB_ID + "}")
+ public Response patchJob(@PathParam(RestConstants.JOB_ID) String id, JobSpec spec) {
+ try {
+ Job job = JobManager.getInstance().patchJob(id, spec);
Review comment:
Again, we need to check if job exists.
----------------------------------------------------------------
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 #249:
SUBMARINE-455. Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
liuxunorg commented on a change in pull request #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#discussion_r404057202
##########
File path: submarine-test/test-k8s/src/test/java/org/apache/submarine/server/client/JobRestApiIT.java
##########
@@ -0,0 +1,182 @@
+/*
+ * 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.client;
+
+import javax.ws.rs.core.MediaType;
+import javax.ws.rs.core.Response;
+import java.io.File;
+import java.io.IOException;
+import java.net.URL;
+import java.nio.charset.StandardCharsets;
+
+import com.google.gson.Gson;
+import com.google.gson.GsonBuilder;
+import org.apache.commons.httpclient.methods.DeleteMethod;
+import org.apache.commons.httpclient.methods.GetMethod;
+import org.apache.commons.httpclient.methods.PostMethod;
+import org.apache.commons.io.FileUtils;
+import org.apache.submarine.server.api.job.Job;
+import org.apache.submarine.server.api.job.JobId;
+import org.apache.submarine.server.api.job.JobServiceException;
+import org.apache.submarine.server.json.JobIdDeserializer;
+import org.apache.submarine.server.json.JobIdSerializer;
+import org.apache.submarine.server.response.JsonResponse;
+import org.apache.submarine.server.rest.RestConstants;
+import org.junit.Assert;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@SuppressWarnings("rawtypes")
+public class JobRestApiIT {
+ private static final Logger LOG = LoggerFactory.getLogger(JobRestApiIT.class);
+
+ private static String JOB_PATH = "/api/" + RestConstants.V1 + "/" + RestConstants.JOBS;
+
+ private Gson gson = new GsonBuilder()
+ .registerTypeAdapter(JobId.class, new JobIdSerializer())
+ .registerTypeAdapter(JobId.class, new JobIdDeserializer())
+ .create();
+
+ @Test
+ public void testJobServerPing() throws IOException {
+ GetMethod response = HttpClientManager.doGet("/api/" + RestConstants.V1 + "/"
+ + RestConstants.JOBS + "/" + RestConstants.PING);
+ String requestBody = response.getResponseBodyAsString();
+ Gson gson = new Gson();
+ JsonResponse jsonResponse = gson.fromJson(requestBody, JsonResponse.class);
+ Assert.assertEquals(Response.Status.OK.getStatusCode(), jsonResponse.getCode());
+ Assert.assertEquals("Pong", jsonResponse.getResult().toString());
+ }
+
+ @Test
+ public void testTensorFlowWithJsonSpec() throws Exception {
+ String body = loadContent("tensorflow/tf-mnist-req.json");
+ String patchBody = loadContent("tensorflow/tf-mnist-patch-req.json");
+ run(body, patchBody, "application/json");
+ }
+
+ @Test
+ public void testTensorFlowWithYamlSpec() throws Exception {
+ String body = loadContent("tensorflow/tf-mnist-req.yaml");
+ String patchBody = loadContent("tensorflow/tf-mnist-patch-req.yaml");
+ run(body, patchBody, "application/yaml");
+ }
+
+ @Test
+ public void testPyTorchWithJsonSpec() throws Exception {
+ String body = loadContent("pytorch/pt-mnist-req.json");
+ String patchBody = loadContent("pytorch/pt-mnist-patch-req.json");
+ run(body, patchBody, "application/json");
+ }
+
+ @Test
+ public void testPyTorchWithYamlSpec() throws Exception {
+ String body = loadContent("pytorch/pt-mnist-req.yaml");
+ String patchBody = loadContent("pytorch/pt-mnist-patch-req.yaml");
+ run(body, patchBody, "application/yaml");
+ }
+
+ private void run(String body, String patchBody, String contentType) throws Exception {
Review comment:
We need to directly obtain the job information in k8s through kubectl api in the `run` function for verification `create job\get job\delete job`.
----------------------------------------------------------------
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] jiwq commented on a change in pull request #249:
SUBMARINE-455. Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
jiwq commented on a change in pull request #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#discussion_r399750799
##########
File path: submarine-server/server-core/src/test/java/org/apache/submarine/server/rest/JobManagerRestApiTest.java
##########
@@ -137,16 +134,12 @@ public void testGetJobById() throws IOException {
.create();
JsonResponse<Job> jsonResponse = gson.fromJson(responseBodyAsString,
new TypeToken<JsonResponse<Job>>(){}.getType());
- assertEquals("Response code should be 200 ",
- Response.Status.OK.getStatusCode(), jsonResponse.getCode());
- assertEquals("Job id should be " + jobId, jobId,
- jsonResponse.getResult().getJobId().toString());
+ assertEquals(Response.Status.NOT_FOUND.getStatusCode(), jsonResponse.getCode());
Review comment:
Hi @tangzhankun , I mocked the Submitter to resolve the Job REST 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] jiwq commented on a change in pull request #249:
SUBMARINE-455. Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
jiwq commented on a change in pull request #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#discussion_r404059162
##########
File path: conf/submarine-site.xml.template
##########
@@ -155,4 +155,28 @@
<description>Rpc server port</description>
</property>
+ <!-- Submarine Submitters Configuration -->
+ <property>
+ <name>submarine.submitters</name>
+ <value>k8s</value>
+ <description>Submitter list for the server</description>
+ </property>
+ <property>
+ <name>submarine.submitters.k8s.class</name>
+ <value>org.apache.submarine.server.submitter.k8s.K8sJobSubmitter</value>
+ <description>The entry class for the specified submitter</description>
+ </property>
+ <property>
+ <name>submarine.submitters.k8s.classpath</name>
Review comment:
As we discussed offline, I got it will be fixed in the next. At now we should config it. In cluster mode, we don't need this.
----------------------------------------------------------------
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 #249:
SUBMARINE-455. Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
liuxunorg commented on a change in pull request #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#discussion_r405428731
##########
File path: submarine-server/server-core/src/main/java/org/apache/submarine/server/job/JobManager.java
##########
@@ -0,0 +1,214 @@
+/*
+ * 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.job;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
+import java.util.concurrent.atomic.AtomicInteger;
+
+import javax.ws.rs.core.Response.Status;
+
+import org.apache.submarine.commons.utils.SubmarineConfiguration;
+import org.apache.submarine.server.SubmarineServer;
+import org.apache.submarine.server.SubmitterManager;
+import org.apache.submarine.server.api.exception.SubmitterException;
+import org.apache.submarine.server.api.job.JobSubmitter;
+import org.apache.submarine.server.api.job.Job;
+import org.apache.submarine.server.api.job.JobId;
+import org.apache.submarine.server.api.spec.JobSpec;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * It responsible for manage the job (CRUD) and cached the job.
+ */
+public class JobManager {
+ private static final Logger LOG = LoggerFactory.getLogger(JobManager.class);
+
+ private static volatile JobManager manager;
+
+ private final AtomicInteger jobCounter = new AtomicInteger(0);
+
+ /**
+ * Used to cache the specs by the job id.
+ * key: the string of job id
+ * value: Job object
+ */
+ private final ConcurrentMap<String, Job> cachedJobMap = new ConcurrentHashMap<>();
+
+ private SubmitterManager submitterManager;
+
+ /**
+ * Get the singleton instance
+ * @return object
+ */
+ public static JobManager getInstance() {
+ if (manager == null) {
+ synchronized (JobManager.class) {
+ if (manager == null) {
+ SubmarineConfiguration conf = SubmarineConfiguration.getInstance();
+ SubmitterManager submitterManager = new SubmitterManager(conf);
+ manager = new JobManager(submitterManager);
+ }
+ }
+ }
+ return manager;
+ }
+
+ private JobManager(SubmitterManager submitterManager) {
+ this.submitterManager = submitterManager;
+ }
+
+ /**
+ * Create job
+ * @param spec job spec
+ * @return object
+ * @throws JobServiceException the service error
+ */
+ public Job createJob(JobSpec spec) throws JobServiceException {
+ try {
+ JobSubmitter submitter = getSubmitter(spec.getSubmitterSpec().getType());
+ Job job = submitter.createJob(spec);
+ job.setJobId(generateJobId());
+ job.setSpec(spec);
+ cachedJobMap.putIfAbsent(job.getJobId().toString(), job);
+ return job;
+ } catch (SubmitterException e) {
+ throw JobServiceException.fromSubmitterException(e);
Review comment:
Now the extended `*Exception` makes the system very complicated.
It is sufficient to use `SubmarineRuntimeException` in the commons-utils module.
----------------------------------------------------------------
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 #249:
SUBMARINE-455. Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
tangzhankun commented on a change in pull request #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#discussion_r405378705
##########
File path: docs/submarine-server/rest-api/job.md
##########
@@ -0,0 +1,323 @@
+<!--
+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.
+-->
+# Submarine Job RESTful API
+## Create Job
+`POST /api/v1/jobs`
+
+**Example Request**
+```sh
+curl -X POST -H "Content-Type: application/json" -d '
+{
+ "name": "mnist",
+ "librarySpec": {
+ "name": "TensorFlow",
+ "version": "2.1.0",
+ "image": "gcr.io/kubeflow-ci/tf-mnist-with-summaries:1.0",
+ "cmd": "python /var/tf_mnist/mnist_with_summaries.py --log_dir=/train/log --learning_rate=0.01 --batch_size=150",
+ "envVars": {
+ "ENV_1": "ENV1"
+ }
+ },
+ "submitterSpec": {
+ "type": "k8s",
+ "namespace": "submarine"
+ },
+ "taskSpecs": {
+ "Worker": {
+ "name": "tensorflow",
+ "replicas": 1,
+ "resources": "cpu=1,memory=1024M"
+ }
+ }
+}
+' http://127.0.0.1/api/v1/jobs
+```
+
+**Example Response:**
+```sh
+{
+ "status": "OK",
+ "code": 200,
+ "result": {
+ "jobId": "job_1586156073228_0005",
+ "name": "mnist",
+ "uid": "28e39dcd-77d4-11ea-8dbb-0242ac110003",
+ "status": "Accepted",
+ "acceptedTime": "2020-04-06T14:59:29.000+08:00",
+ "spec": {
+ "name": "mnist",
+ "librarySpec": {
+ "name": "TensorFlow",
+ "version": "2.1.0",
+ "image": "gcr.io/kubeflow-ci/tf-mnist-with-summaries:1.0",
+ "cmd": "python /var/tf_mnist/mnist_with_summaries.py --log_dir\u003d/train/log --learning_rate\u003d0.01 --batch_size\u003d150",
+ "envVars": {
+ "ENV_1": "ENV1"
+ }
+ },
+ "submitterSpec": {
+ "type": "k8s",
+ "namespace": "submarine"
+ },
+ "taskSpecs": {
+ "Worker": {
+ "name": "tensorflow",
+ "resources": "cpu\u003d1,memory\u003d1024M",
Review comment:
What's the "\u003d1"?
----------------------------------------------------------------
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] jiwq commented on a change in pull request #249:
SUBMARINE-455. Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
jiwq commented on a change in pull request #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#discussion_r404065860
##########
File path: submarine-cloud/hack/deploy-submarine.sh
##########
@@ -57,7 +67,10 @@ function install_submarine() {
if $KUBECTL_BIN get configmap --namespace default | grep submarine-config >/dev/null ; then
$KUBECTL_BIN delete configmap --namespace default submarine-config
fi
- $KUBECTL_BIN create configmap --namespace default submarine-config --from-file=${ROOT}/hack/conf/submarine-site.xml --from-file=${ROOT}/hack/conf/log4j.properties
+ $KUBECTL_BIN create configmap --namespace default submarine-config \
+ --from-file=${ROOT}/hack/conf/submarine-site.xml \
+ --from-file=${ROOT}/hack/conf/log4j.properties \
+ --from-file=${ROOT}/hack/conf/k8s/config
Review comment:
Sorry for that, I forgot to remove it. In cluster mode, we don't need this.
----------------------------------------------------------------
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 #249:
SUBMARINE-455. Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
tangzhankun commented on a change in pull request #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#discussion_r405374725
##########
File path: dev-support/k8s/deploy-kubeflow-operators.sh
##########
@@ -0,0 +1,161 @@
+#!/bin/bash
+#
+# 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.
+#
+set -e
+
+readonly TF_OPERATOR_IMAGE="gcr.io/kubeflow-images-public/tf_operator:v1.0.0-g92389064"
+readonly PYTORCH_OPERATOR_IMAGE="gcr.io/kubeflow-images-public/pytorch-operator:v1.0.0-g047cf0f"
+readonly TF_MNIST_IMAGE="gcr.io/kubeflow-ci/tf-mnist-with-summaries:1.0"
+readonly PT_MNIST_IMAGE="apache/submarine:pytorch-dist-mnist-1.0"
+
+if [ -L "${BASH_SOURCE-$0}" ]; then
+ PWD=$(dirname "$(readlink "${BASH_SOURCE-$0}")")
+else
+ PWD=$(dirname "${BASH_SOURCE-$0}")
+fi
+CURRENT_PATH=$(cd "${PWD}">/dev/null; pwd)
+export CURRENT_PATH
+export SUBMARINE_HOME=${CURRENT_PATH}/../..
+# lib.sh use the ROOT variable
+export ROOT="${SUBMARINE_HOME}/submarine-cloud/"
+export KUBECONFIG="${HOME}/.kube/kind-config-${clusterName:-kind}"
+
+# shellcheck source=./../../submarine-cloud/hack/lib.sh
+source "${SUBMARINE_HOME}/submarine-cloud/hack/lib.sh"
+
+###########################################
+# Load local docker image into K8s registry
+# Globals:
+# KIND_BIN
+# Arguments:
+# image
+###########################################
+function load_image_to_k8s() {
Review comment:
Is "load_image_to_kind" more accurate? And in this script, all "k8s" seems to be "kind"?
----------------------------------------------------------------
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 #249:
SUBMARINE-455. Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
tangzhankun commented on a change in pull request #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#discussion_r399739761
##########
File path: submarine-server/server-submitter/submitter-k8s/src/main/java/org/apache/submarine/server/submitter/k8s/mapper/MLJobMapper.java
##########
@@ -0,0 +1,93 @@
+/*
+ * 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.submitter.k8s.mapper;
+
+import java.util.List;
+
+import io.kubernetes.client.models.V1DeleteOptions;
+import io.kubernetes.client.models.V1DeleteOptionsBuilder;
+import io.kubernetes.client.models.V1JobCondition;
+import io.kubernetes.client.models.V1JobStatus;
+import io.kubernetes.client.models.V1Status;
+import io.kubernetes.client.models.V1StatusDetails;
+import org.apache.submarine.server.api.job.Job;
+import org.apache.submarine.server.submitter.k8s.model.MLJob;
+import org.joda.time.DateTime;
+
+public class MLJobMapper {
Review comment:
Please comment on the purpose of this and how it works generally.
----------------------------------------------------------------
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 #249:
SUBMARINE-455. Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
liuxunorg commented on a change in pull request #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#discussion_r398277405
##########
File path: submarine-server/server-submitter/submitter-k8s/src/main/java/org/apache/submarine/server/submitter/k8s/K8sJobSubmitter.java
##########
@@ -101,98 +101,78 @@ public String getSubmitterType() {
}
@Override
- public Job submitJob(JobSpec jobSpec)
- throws InvalidSpecException {
+ public Job createJob(JobSpec jobSpec) throws InvalidSpecException {
Job job = null;
-
- boolean success = createJob(JobSpecParser.parseJob(jobSpec));
- if (success) {
- job = new Job();
- job.setName(jobSpec.getName());
- } else {
- LOG.error("Failed to create job." + jobSpec.toString());
+ try {
+ MLJob mlJob = JobSpecParser.parseJob(jobSpec);
+ Object object = api.createNamespacedCustomObject(mlJob.getGroup(), mlJob.getVersion(),
+ mlJob.getMetadata().getNamespace(), mlJob.getPlural(), mlJob, "true");
+ job = parseResponseObject(object, ParseOp.PARSE_OP_RESULT);
+ } catch (ApiException e) {
+ LOG.error("Failed to create job. " + e.getMessage(), e);
}
return job;
}
- @VisibleForTesting
- boolean createJob(MLJob job) {
+ @Override
+ public Job findJob(JobSpec jobSpec) throws InvalidSpecException {
+ Job job = null;
try {
- api.createNamespacedCustomObject(job.getGroup(), job.getVersion(),
- job.getMetadata().getNamespace(), job.getPlural(),
- job, "true");
+ MLJob mlJob = JobSpecParser.parseJob(jobSpec);
+ Object object = api.getNamespacedCustomObject(mlJob.getGroup(), mlJob.getVersion(),
+ mlJob.getMetadata().getNamespace(), mlJob.getPlural(), mlJob.getMetadata().getName());
+ job = parseResponseObject(object, ParseOp.PARSE_OP_RESULT);
} catch (ApiException e) {
- LOG.error("Failed to create job. " + e.getMessage(), e);
- return false;
+ LOG.warn("Failed to get job. " + e.getMessage(), e);
}
- return true;
+ return job;
}
- @VisibleForTesting
- CustomResourceJob createCustomJob(K8sJobRequest request) {
+ @Override
+ public Job patchJob(JobSpec jobSpec) throws InvalidSpecException {
+ Job job = null;
try {
- K8sJobRequest.Path path = request.getPath();
- Object o = api.createNamespacedCustomObject(path.getGroup(),
- path.getApiVersion(), path.getNamespace(), path.getPlural(),
- request.getBody(), "true");
- Gson gson = new Gson();
- return gson.fromJson(gson.toJson(o), CustomResourceJob.class);
- } catch (ApiException ae) {
- LOG.error("Exceptions when creating CRD job: " + ae.getMessage(), ae);
+ MLJob mlJob = JobSpecParser.parseJob(jobSpec);
+ Object object = api.patchNamespacedCustomObject(mlJob.getGroup(), mlJob.getVersion(),
+ mlJob.getMetadata().getNamespace(), mlJob.getPlural(), mlJob.getMetadata().getName(),
+ mlJob);
+ job = parseResponseObject(object, ParseOp.PARSE_OP_RESULT);
+ } catch (ApiException e) {
+ LOG.warn("Failed to update job. " + e.getMessage(), e);
}
- return null;
+ return job;
}
- @VisibleForTesting
- CustomResourceJob getCustomResourceJob(K8sJobRequest request) {
+ @Override
+ public Job deleteJob(JobSpec jobSpec) {
+ Job job = null;
try {
- K8sJobRequest.Path path = request.getPath();
- Object o = api.getNamespacedCustomObject(path.getGroup(),
- path.getApiVersion(),
- path.getNamespace(), path.getPlural(), request.getJobName());
- Gson gson = new Gson();
- return gson.fromJson(gson.toJson(o), CustomResourceJob.class);
- } catch (ApiException ae) {
- // The API getNamespacedCustomObject throws exception when cannot found resource
- // So the ApiException seems not a big issue
- LOG.warn("Exceptions when getting CRD job: " + ae.getMessage());
+ MLJob mlJob = JobSpecParser.parseJob(jobSpec);
+ Object object = api.deleteNamespacedCustomObject(mlJob.getGroup(), mlJob.getVersion(),
Review comment:
I think, you should first determine whether the job name does not exist, and return an accurate error message if it does not exist.
----------------------------------------------------------------
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] jiwq commented on issue #249: SUBMARINE-455. Support
find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
jiwq commented on issue #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#issuecomment-609686063
Hi @liuxunorg @tangzhankun @yuanzac , thanks for your review. I had resolved all comments and add the integration test. Please help to review. The travis error is not related to the change, it caused by `The job exceeded the maximum time limit for jobs, and has been terminated.`
----------------------------------------------------------------
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 #249:
SUBMARINE-455. Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
liuxunorg commented on a change in pull request #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#discussion_r404016197
##########
File path: conf/submarine-site.xml.template
##########
@@ -155,4 +155,28 @@
<description>Rpc server port</description>
</property>
+ <!-- Submarine Submitters Configuration -->
+ <property>
+ <name>submarine.submitters</name>
Review comment:
Change `submarine.submitters` to `submarine.submitter`.
Submarine can only support one submission type at the same time
----------------------------------------------------------------
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] jiwq commented on a change in pull request #249:
SUBMARINE-455. Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
jiwq commented on a change in pull request #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#discussion_r404065951
##########
File path: conf/submarine-site.xml.template
##########
@@ -155,4 +155,28 @@
<description>Rpc server port</description>
</property>
+ <!-- Submarine Submitters Configuration -->
+ <property>
+ <name>submarine.submitters</name>
+ <value>k8s</value>
+ <description>Submitter list for the server</description>
+ </property>
+ <property>
+ <name>submarine.submitters.k8s.class</name>
+ <value>org.apache.submarine.server.submitter.k8s.K8sJobSubmitter</value>
+ <description>The entry class for the specified submitter</description>
+ </property>
+ <property>
+ <name>submarine.submitters.k8s.classpath</name>
+ <value>../lib/submitter/k8s/</value>
+ <description>The libs for Kubernetes submitter</description>
+ </property>
+
+ <!-- K8s Configuration -->
+ <property>
+ <name>submarine.k8s.kube.config</name>
Review comment:
As we discussed offline, I got it will be fixed in the next. At now we should config it. But in cluster mode, we don't need this.
----------------------------------------------------------------
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 #249:
SUBMARINE-455. Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
liuxunorg commented on a change in pull request #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#discussion_r398275581
##########
File path: submarine-server/server-core/src/test/java/org/apache/submarine/server/rest/JobManagerRestApiTest.java
##########
@@ -137,16 +134,12 @@ public void testGetJobById() throws IOException {
.create();
JsonResponse<Job> jsonResponse = gson.fromJson(responseBodyAsString,
new TypeToken<JsonResponse<Job>>(){}.getType());
- assertEquals("Response code should be 200 ",
- Response.Status.OK.getStatusCode(), jsonResponse.getCode());
- assertEquals("Job id should be " + jobId, jobId,
- jsonResponse.getResult().getJobId().toString());
+ assertEquals(Response.Status.NOT_FOUND.getStatusCode(), jsonResponse.getCode());
Review comment:
`testGetJobById()` needs to be able to get the JOB after submitting the JOB instead of `NOT_FOUND` job, otherwise this test case is meaningless.
----------------------------------------------------------------
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 issue #249: SUBMARINE-455.
Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
tangzhankun commented on issue #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#issuecomment-605758724
@jiwq One thing forgets to mention, the submarine server document needs an update too.
----------------------------------------------------------------
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 edited a comment on issue #249:
SUBMARINE-455. Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
tangzhankun edited a comment on issue #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#issuecomment-611301167
@jiwq , Thanks for the great contribution! It looks good to me. +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] tangzhankun commented on a change in pull request #249:
SUBMARINE-455. Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
tangzhankun commented on a change in pull request #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#discussion_r405395648
##########
File path: submarine-server/server-submitter/submitter-k8s/src/main/java/org/apache/submarine/server/submitter/k8s/util/MLJobConverter.java
##########
@@ -0,0 +1,97 @@
+/*
+ * 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.submitter.k8s.util;
+
+import java.util.List;
+
+import io.kubernetes.client.models.V1DeleteOptions;
+import io.kubernetes.client.models.V1DeleteOptionsBuilder;
+import io.kubernetes.client.models.V1JobCondition;
+import io.kubernetes.client.models.V1JobStatus;
+import io.kubernetes.client.models.V1Status;
+import io.kubernetes.client.models.V1StatusDetails;
+import org.apache.submarine.server.api.job.Job;
+import org.apache.submarine.server.submitter.k8s.model.MLJob;
+import org.joda.time.DateTime;
+
+/**
+ * Mappings between Java bean types.
Review comment:
"Converter for different types." ?
----------------------------------------------------------------
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] jiwq commented on a change in pull request #249:
SUBMARINE-455. Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
jiwq commented on a change in pull request #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#discussion_r404059397
##########
File path: conf/submarine-site.xml.template
##########
@@ -155,4 +155,28 @@
<description>Rpc server port</description>
</property>
+ <!-- Submarine Submitters Configuration -->
+ <property>
+ <name>submarine.submitters</name>
Review comment:
As we discussed offline, I got it will be fixed in the next. At now we should config 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] tangzhankun commented on a change in pull request #249:
SUBMARINE-455. Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
tangzhankun commented on a change in pull request #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#discussion_r405399875
##########
File path: submarine-test/test-k8s/src/test/resources/pytorch/pt-mnist-patch-req.json
##########
@@ -0,0 +1,28 @@
+{
+ "name": "mnist",
+ "librarySpec": {
+ "name": "pytorch",
+ "version": "2.1.0",
+ "image": "apache/submarine:pytorch-dist-mnist-1.0",
+ "cmd": "python /var/mnist.py --backend gloo",
+ "envVars": {
+ "ENV_1": "ENV1"
+ }
+ },
+ "submitterSpec": {
+ "type": "k8s",
Review comment:
As discussed offline, let's change this job spec in next PR
----------------------------------------------------------------
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] jiwq commented on a change in pull request #249:
SUBMARINE-455. Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
jiwq commented on a change in pull request #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#discussion_r399796570
##########
File path: submarine-server/server-submitter/submitter-k8s/src/main/java/org/apache/submarine/server/submitter/k8s/mapper/MLJobMapper.java
##########
@@ -0,0 +1,93 @@
+/*
+ * 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.submitter.k8s.mapper;
+
+import java.util.List;
+
+import io.kubernetes.client.models.V1DeleteOptions;
+import io.kubernetes.client.models.V1DeleteOptionsBuilder;
+import io.kubernetes.client.models.V1JobCondition;
+import io.kubernetes.client.models.V1JobStatus;
+import io.kubernetes.client.models.V1Status;
+import io.kubernetes.client.models.V1StatusDetails;
+import org.apache.submarine.server.api.job.Job;
+import org.apache.submarine.server.submitter.k8s.model.MLJob;
+import org.joda.time.DateTime;
+
+public class MLJobMapper {
Review comment:
Done.
----------------------------------------------------------------
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 #249:
SUBMARINE-455. Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
tangzhankun commented on a change in pull request #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#discussion_r405399018
##########
File path: submarine-test/test-k8s/src/test/java/org/apache/submarine/server/client/JobManagerRestApiIT.java
##########
@@ -0,0 +1,235 @@
+/*
+ * 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.client;
+
+import javax.ws.rs.core.MediaType;
+import javax.ws.rs.core.Response;
+import java.io.File;
+import java.io.FileReader;
+import java.io.IOException;
+import java.net.URL;
+import java.nio.charset.StandardCharsets;
+
+import com.google.gson.Gson;
+import com.google.gson.GsonBuilder;
+import com.google.gson.JsonObject;
+import io.kubernetes.client.ApiClient;
+import io.kubernetes.client.ApiException;
+import io.kubernetes.client.Configuration;
+import io.kubernetes.client.JSON;
+import io.kubernetes.client.apis.CustomObjectsApi;
+import io.kubernetes.client.util.ClientBuilder;
+import io.kubernetes.client.util.KubeConfig;
+import org.apache.commons.httpclient.methods.DeleteMethod;
+import org.apache.commons.httpclient.methods.GetMethod;
+import org.apache.commons.httpclient.methods.PostMethod;
+import org.apache.commons.io.FileUtils;
+import org.apache.submarine.server.api.job.Job;
+import org.apache.submarine.server.api.job.JobId;
+import org.apache.submarine.server.json.JobIdDeserializer;
+import org.apache.submarine.server.json.JobIdSerializer;
+import org.apache.submarine.server.response.JsonResponse;
+import org.apache.submarine.server.rest.RestConstants;
+import org.joda.time.DateTime;
+import org.junit.Assert;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@SuppressWarnings("rawtypes")
+public class JobManagerRestApiIT {
+ private static final Logger LOG = LoggerFactory.getLogger(JobManagerRestApiIT.class);
+
+ private static String JOB_PATH = "/api/" + RestConstants.V1 + "/" + RestConstants.JOBS;
+
+ private Gson gson = new GsonBuilder()
+ .registerTypeAdapter(JobId.class, new JobIdSerializer())
+ .registerTypeAdapter(JobId.class, new JobIdDeserializer())
+ .create();
+
+ @Test
+ public void testJobServerPing() throws IOException {
+ GetMethod response = HttpClientManager.doGet("/api/" + RestConstants.V1 + "/"
+ + RestConstants.JOBS + "/" + RestConstants.PING);
+ String requestBody = response.getResponseBodyAsString();
+ Gson gson = new Gson();
+ JsonResponse jsonResponse = gson.fromJson(requestBody, JsonResponse.class);
+ Assert.assertEquals(Response.Status.OK.getStatusCode(), jsonResponse.getCode());
+ Assert.assertEquals("Pong", jsonResponse.getResult().toString());
+ }
+
+ @Test
+ public void testTensorFlowWithJsonSpec() throws Exception {
+ String body = loadContent("tensorflow/tf-mnist-req.json");
+ String patchBody = loadContent("tensorflow/tf-mnist-patch-req.json");
+ run(body, patchBody, "application/json");
+ }
+
+ @Test
+ public void testTensorFlowWithYamlSpec() throws Exception {
+ String body = loadContent("tensorflow/tf-mnist-req.yaml");
+ String patchBody = loadContent("tensorflow/tf-mnist-patch-req.yaml");
+ run(body, patchBody, "application/yaml");
+ }
+
+ @Test
+ public void testPyTorchWithJsonSpec() throws Exception {
+ String body = loadContent("pytorch/pt-mnist-req.json");
+ String patchBody = loadContent("pytorch/pt-mnist-patch-req.json");
+ run(body, patchBody, "application/json");
+ }
+
+ @Test
+ public void testPyTorchWithYamlSpec() throws Exception {
+ String body = loadContent("pytorch/pt-mnist-req.yaml");
+ String patchBody = loadContent("pytorch/pt-mnist-patch-req.yaml");
+ run(body, patchBody, "application/yaml");
+ }
+
+ private void run(String body, String patchBody, String contentType) throws Exception {
+ // create
+ PostMethod postMethod = HttpClientManager.doPost(JOB_PATH, body, contentType);
+ Assert.assertEquals(Response.Status.OK.getStatusCode(), postMethod.getStatusCode());
+
+ String json = postMethod.getResponseBodyAsString();
+ JsonResponse jsonResponse = gson.fromJson(json, JsonResponse.class);
+ Assert.assertEquals(Response.Status.OK.getStatusCode(), jsonResponse.getCode());
+
+ Job createdJob = gson.fromJson(gson.toJson(jsonResponse.getResult()), Job.class);
+ Assert.assertEquals("mnist", createdJob.getName());
Review comment:
We need to verify more fields in the job. Just the name and the response code is not enough.
For instance, we can verify the "uid" and the "acceptedTime" is not null because they are populated by the K8s.
----------------------------------------------------------------
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 #249:
SUBMARINE-455. Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
liuxunorg commented on a change in pull request #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#discussion_r404017707
##########
File path: submarine-test/test-k8s/src/test/resources/tensorflow/tf-mnist-patch-req.json
##########
@@ -0,0 +1,23 @@
+{
+ "name": "mnist",
+ "librarySpec": {
+ "name": "TensorFlow",
+ "version": "2.1.0",
+ "image": "gcr.io/kubeflow-ci/tf-mnist-with-summaries:1.0",
+ "cmd": "python /var/tf_mnist/mnist_with_summaries.py --log_dir=/train/log --learning_rate=0.01 --batch_size=150",
+ "envVars": {
+ "ENV_1": "ENV1"
+ }
+ },
+ "submitterSpec": {
+ "type": "k8s",
Review comment:
We don't need
```type: "k8s"```
The submission to `k8s` or `yarn` is determined by submarine, not by the user.
----------------------------------------------------------------
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] asfgit closed pull request #249: SUBMARINE-455. Support
CRUD job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #249: SUBMARINE-455. Support CRUD job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249
----------------------------------------------------------------
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 #249:
SUBMARINE-455. Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
liuxunorg commented on a change in pull request #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#discussion_r404033915
##########
File path: submarine-cloud/hack/output/bin/kind
##########
@@ -0,0 +1 @@
+/usr/local/bin/kind
Review comment:
Need delete `submarine-cloud/hack/output/bin/kind` file.
kind will automatically download.
----------------------------------------------------------------
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 #249:
SUBMARINE-455. Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
liuxunorg commented on a change in pull request #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#discussion_r404017833
##########
File path: submarine-test/test-k8s/src/test/resources/pytorch/pt-mnist-req.yaml
##########
@@ -0,0 +1,21 @@
+---
+name: mnist
+librarySpec:
+ name: pytorch
+ version: 2.1.0
+ image: apache/submarine:pytorch-dist-mnist-1.0
+ cmd: python /var/mnist.py --backend gloo
+ envVars:
+ ENV_1: ENV1
+submitterSpec:
+ type: k8s
Review comment:
We don't need
```type: k8s```
The submission to `k8s` or `yarn` is determined by submarine, not by the user.
----------------------------------------------------------------
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] jiwq commented on a change in pull request #249:
SUBMARINE-455. Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
jiwq commented on a change in pull request #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#discussion_r399752515
##########
File path: submarine-server/server-core/src/main/java/org/apache/submarine/server/JobManager.java
##########
@@ -45,10 +47,9 @@
private final AtomicInteger jobCounter = new AtomicInteger(0);
- private final ConcurrentMap<JobId, Job> jobs = new ConcurrentHashMap<>();
+ private final ConcurrentMap<String, JobSpec> cachedJobSpecs = new ConcurrentHashMap<>();
Review comment:
Done
----------------------------------------------------------------
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] jiwq commented on a change in pull request #249:
SUBMARINE-455. Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
jiwq commented on a change in pull request #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#discussion_r405544690
##########
File path: dev-support/k8s/deploy-kubeflow-operators.sh
##########
@@ -0,0 +1,161 @@
+#!/bin/bash
+#
+# 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.
+#
+set -e
+
+readonly TF_OPERATOR_IMAGE="gcr.io/kubeflow-images-public/tf_operator:v1.0.0-g92389064"
+readonly PYTORCH_OPERATOR_IMAGE="gcr.io/kubeflow-images-public/pytorch-operator:v1.0.0-g047cf0f"
+readonly TF_MNIST_IMAGE="gcr.io/kubeflow-ci/tf-mnist-with-summaries:1.0"
+readonly PT_MNIST_IMAGE="apache/submarine:pytorch-dist-mnist-1.0"
+
+if [ -L "${BASH_SOURCE-$0}" ]; then
+ PWD=$(dirname "$(readlink "${BASH_SOURCE-$0}")")
+else
+ PWD=$(dirname "${BASH_SOURCE-$0}")
+fi
+CURRENT_PATH=$(cd "${PWD}">/dev/null; pwd)
+export CURRENT_PATH
+export SUBMARINE_HOME=${CURRENT_PATH}/../..
+# lib.sh use the ROOT variable
+export ROOT="${SUBMARINE_HOME}/submarine-cloud/"
+export KUBECONFIG="${HOME}/.kube/kind-config-${clusterName:-kind}"
+
+# shellcheck source=./../../submarine-cloud/hack/lib.sh
+source "${SUBMARINE_HOME}/submarine-cloud/hack/lib.sh"
+
+###########################################
+# Load local docker image into K8s registry
+# Globals:
+# KIND_BIN
+# Arguments:
+# image
+###########################################
+function load_image_to_k8s() {
Review comment:
Done.
----------------------------------------------------------------
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 #249:
SUBMARINE-455. Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
liuxunorg commented on a change in pull request #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#discussion_r404024147
##########
File path: submarine-test/test-k8s/src/test/java/org/apache/submarine/server/client/JobRestApiIT.java
##########
@@ -0,0 +1,182 @@
+/*
+ * 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.client;
+
+import javax.ws.rs.core.MediaType;
+import javax.ws.rs.core.Response;
+import java.io.File;
+import java.io.IOException;
+import java.net.URL;
+import java.nio.charset.StandardCharsets;
+
+import com.google.gson.Gson;
+import com.google.gson.GsonBuilder;
+import org.apache.commons.httpclient.methods.DeleteMethod;
+import org.apache.commons.httpclient.methods.GetMethod;
+import org.apache.commons.httpclient.methods.PostMethod;
+import org.apache.commons.io.FileUtils;
+import org.apache.submarine.server.api.job.Job;
+import org.apache.submarine.server.api.job.JobId;
+import org.apache.submarine.server.api.job.JobServiceException;
+import org.apache.submarine.server.json.JobIdDeserializer;
+import org.apache.submarine.server.json.JobIdSerializer;
+import org.apache.submarine.server.response.JsonResponse;
+import org.apache.submarine.server.rest.RestConstants;
+import org.junit.Assert;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@SuppressWarnings("rawtypes")
+public class JobRestApiIT {
Review comment:
public class JobRestApiIT extends AbstractSubmarineServerTest {
----------------------------------------------------------------
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 #249:
SUBMARINE-455. Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
tangzhankun commented on a change in pull request #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#discussion_r405383765
##########
File path: docs/submarine-server/README.md
##########
@@ -198,6 +198,8 @@ curl -X POST -H "Content-Type: application/json" -d '
' http://127.0.0.1:8080/api/v1/jobs
```
+For more info about Job RESTful API click [here](./rest-api/job.md).
Review comment:
Can we put the content of "job.md" to "README.md"? And please remove the "kubectl" way in README.md. That command should be removed since we have the API way to verify and delete jobs.
----------------------------------------------------------------
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 #249:
SUBMARINE-455. Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
liuxunorg commented on a change in pull request #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#discussion_r405428955
##########
File path: submarine-server/server-core/src/main/java/org/apache/submarine/server/job/JobServiceNotFoundJobException.java
##########
@@ -17,12 +17,10 @@
* under the License.
*/
-package org.apache.submarine.server.api.exception;
+package org.apache.submarine.server.job;
-public class UnsupportedJobTypeException extends Exception {
- private static final long serialVersionUID = 4752254162145918312L;
-
- public UnsupportedJobTypeException() {
- super("Unsupported Job Type Exception");
+public class JobServiceNotFoundJobException extends JobServiceException {
+ public JobServiceNotFoundJobException() {
Review comment:
Now the extended `*Exception` makes the system very complicated.
It is sufficient to use `SubmarineRuntimeException` in the commons-utils module.
----------------------------------------------------------------
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 #249:
SUBMARINE-455. Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
tangzhankun commented on a change in pull request #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#discussion_r405392289
##########
File path: submarine-server/server-core/src/main/java/org/apache/submarine/server/rest/JobManagerRestApi.java
##########
@@ -19,32 +19,32 @@
package org.apache.submarine.server.rest;
-import org.apache.submarine.server.JobManager;
-import org.apache.submarine.server.api.exception.UnsupportedJobTypeException;
-import org.apache.submarine.server.api.job.Job;
-import org.apache.submarine.server.api.job.JobId;
-import org.apache.submarine.server.api.spec.JobSpec;
-import org.apache.submarine.server.response.JsonResponse;
-
import javax.ws.rs.Consumes;
import javax.ws.rs.DELETE;
import javax.ws.rs.GET;
+import javax.ws.rs.PATCH;
import javax.ws.rs.POST;
import javax.ws.rs.Path;
import javax.ws.rs.PathParam;
import javax.ws.rs.Produces;
+import javax.ws.rs.QueryParam;
import javax.ws.rs.core.MediaType;
import javax.ws.rs.core.Response;
-import java.util.ArrayList;
import java.util.List;
+import org.apache.submarine.server.job.JobManager;
+import org.apache.submarine.server.api.job.Job;
+import org.apache.submarine.server.job.JobServiceException;
+import org.apache.submarine.server.api.spec.JobSpec;
+import org.apache.submarine.server.response.JsonResponse;
+
/**
- * {@link JobManager}'s REST API v1. It can accept {@link JobSpec} to create a job.
+ * Job Service REST API v1. It can accept {@link JobSpec} to create a job.
*/
@Path(RestConstants.V1 + "/" + RestConstants.JOBS)
@Produces({MediaType.APPLICATION_JSON + "; " + RestConstants.CHARSET_UTF8})
public class JobManagerRestApi {
-
+ private final JobManager service = JobManager.getInstance();
Review comment:
Please use a more meaningful name like "jobManager" instead of "service", the name "service" is unclear.
----------------------------------------------------------------
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 #249:
SUBMARINE-455. Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
tangzhankun commented on a change in pull request #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#discussion_r399738437
##########
File path: submarine-server/server-core/src/main/java/org/apache/submarine/server/JobManager.java
##########
@@ -45,10 +47,9 @@
private final AtomicInteger jobCounter = new AtomicInteger(0);
- private final ConcurrentMap<JobId, Job> jobs = new ConcurrentHashMap<>();
+ private final ConcurrentMap<String, JobSpec> cachedJobSpecs = new ConcurrentHashMap<>();
Review comment:
Could you please put a comment here explaining the meaning of the key in cachedJobSpecs? Adding a comment to the key struct will make it easier to read.
----------------------------------------------------------------
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 #249:
SUBMARINE-455. Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
liuxunorg commented on a change in pull request #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#discussion_r404017885
##########
File path: submarine-test/test-k8s/src/test/resources/pytorch/pt-mnist-req.json
##########
@@ -0,0 +1,28 @@
+{
+ "name": "mnist",
+ "librarySpec": {
+ "name": "pytorch",
+ "version": "2.1.0",
+ "image": "apache/submarine:pytorch-dist-mnist-1.0",
+ "cmd": "python /var/mnist.py --backend gloo",
+ "envVars": {
+ "ENV_1": "ENV1"
+ }
+ },
+ "submitterSpec": {
+ "type": "k8s",
Review comment:
We don't need
```type: "k8s"```
The submission to `k8s` or `yarn` is determined by submarine, not by the user.
----------------------------------------------------------------
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 a change in pull request #249:
SUBMARINE-455. Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
yuanzac commented on a change in pull request #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#discussion_r399081487
##########
File path: submarine-server/server-submitter/submitter-k8s/src/main/java/org/apache/submarine/server/submitter/k8s/K8sJobSubmitter.java
##########
@@ -59,6 +58,7 @@
public K8sJobSubmitter() {}
+ @VisibleForTesting
Review comment:
Since K8sJobSubmitter is public, we don't need to add "@VisibleForTesting"
----------------------------------------------------------------
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 #249:
SUBMARINE-455. Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
liuxunorg commented on a change in pull request #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#discussion_r398275780
##########
File path: submarine-server/server-core/src/test/java/org/apache/submarine/server/rest/JobManagerRestApiTest.java
##########
@@ -156,8 +149,7 @@ public void testDeleteJobById() throws IOException {
Gson gson = new Gson();
JsonResponse jsonResponse = gson.fromJson(requestBody, JsonResponse.class);
- assertEquals("Response code should be 200 ",
- Response.Status.OK.getStatusCode(), jsonResponse.getCode());
+ assertEquals(Response.Status.NOT_FOUND.getStatusCode(), jsonResponse.getCode());
Review comment:
`testDeleteJobById()` needs to be able to real delete the JOB instead of `NOT_FOUND` JOB, otherwise this test case is meaningless.
----------------------------------------------------------------
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] jiwq commented on a change in pull request #249:
SUBMARINE-455. Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
jiwq commented on a change in pull request #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#discussion_r404064189
##########
File path: submarine-test/test-k8s/src/test/resources/tensorflow/tf-mnist-req.yaml
##########
@@ -0,0 +1,22 @@
+name: "mnist"
+librarySpec:
+ name: "TensorFlow"
+ version: "2.1.0"
+ image: "gcr.io/kubeflow-ci/tf-mnist-with-summaries:1.0"
+ cmd: "python /var/tf_mnist/mnist_with_summaries.py --log_dir=/train/log --learning_rate=0.01 --batch_size=150"
+ envVars:
+ ENV_1: "ENV1"
+submitterSpec:
+ type: "k8s"
Review comment:
As we discussed offline, I got it will be fixed in the next. At now we should config 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] liuxunorg commented on a change in pull request #249:
SUBMARINE-455. Support find/patch/delete job in submarine-server REST
Posted by GitBox <gi...@apache.org>.
liuxunorg commented on a change in pull request #249: SUBMARINE-455. Support find/patch/delete job in submarine-server REST
URL: https://github.com/apache/submarine/pull/249#discussion_r404017423
##########
File path: submarine-test/test-k8s/src/test/resources/tensorflow/tf-mnist-req.yaml
##########
@@ -0,0 +1,22 @@
+name: "mnist"
+librarySpec:
+ name: "TensorFlow"
+ version: "2.1.0"
+ image: "gcr.io/kubeflow-ci/tf-mnist-with-summaries:1.0"
+ cmd: "python /var/tf_mnist/mnist_with_summaries.py --log_dir=/train/log --learning_rate=0.01 --batch_size=150"
+ envVars:
+ ENV_1: "ENV1"
+submitterSpec:
+ type: "k8s"
Review comment:
We don't need
```type: "k8s"```
The submission to `k8s` or `yarn` is determined by submarine, not by the user.
----------------------------------------------------------------
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