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


With regards,
Apache Git Services

Reply via email to