Expose some useful exceptions from LaunchJob

KRAhn · Aug 6, 2019 · 2e82f46 · 2e82f46
1 parent 23ada6b
commit 2e82f46
Show file tree

Hide file tree

Showing 3 changed files with 44 additions and 14 deletions.
diff --git a/genie-web/src/main/java/com/netflix/genie/web/services/JobLaunchService.java b/genie-web/src/main/java/com/netflix/genie/web/services/JobLaunchService.java
@@ -17,8 +17,11 @@
  */
 package com.netflix.genie.web.services;
 
+import com.netflix.genie.common.dto.JobStatus;
 import com.netflix.genie.web.dtos.JobSubmission;
 import com.netflix.genie.web.exceptions.checked.AgentLaunchException;
+import com.netflix.genie.web.exceptions.checked.IdAlreadyExistsException;
+import com.netflix.genie.web.exceptions.checked.SaveAttachmentException;
 import org.springframework.validation.annotation.Validated;
 
 import javax.annotation.Nonnull;
@@ -43,10 +46,14 @@ public interface JobLaunchService {
      *
      * @param jobSubmission The payload of metadata and resources making up all the information needed to launch
      *                      a job
-     * @return The id of the job. Upon return the job will at least be in
-     * {@link com.netflix.genie.common.dto.JobStatus#ACCEPTED} state
-     * @throws AgentLaunchException If the system was unable to launch an agent to handle job execution
+     * @return The id of the job. Upon return the job will at least be in {@link JobStatus#ACCEPTED} state
+     * @throws AgentLaunchException     If the system was unable to launch an agent to handle job execution
+     * @throws IdAlreadyExistsException If the unique identifier for the job conflicts with an already existing job
+     * @throws SaveAttachmentException  When a job is submitted with attachments but there is an error saving them
      */
     @Nonnull
-    String launchJob(@Valid JobSubmission jobSubmission) throws AgentLaunchException;
+    String launchJob(@Valid JobSubmission jobSubmission) throws
+        AgentLaunchException,
+        IdAlreadyExistsException,
+        SaveAttachmentException;
 }
diff --git a/genie-web/src/main/java/com/netflix/genie/web/services/impl/JobLaunchServiceImpl.java b/genie-web/src/main/java/com/netflix/genie/web/services/impl/JobLaunchServiceImpl.java
@@ -24,6 +24,8 @@
 import com.netflix.genie.web.dtos.JobSubmission;
 import com.netflix.genie.web.dtos.ResolvedJob;
 import com.netflix.genie.web.exceptions.checked.AgentLaunchException;
+import com.netflix.genie.web.exceptions.checked.IdAlreadyExistsException;
+import com.netflix.genie.web.exceptions.checked.SaveAttachmentException;
 import com.netflix.genie.web.services.JobLaunchService;
 import com.netflix.genie.web.services.JobResolverService;
 import com.netflix.genie.web.util.MetricsUtils;
@@ -77,7 +79,9 @@ public JobLaunchServiceImpl(
      */
     @Override
     @Nonnull
-    public String launchJob(@Valid final JobSubmission jobSubmission) throws AgentLaunchException {
+    public String launchJob(
+        @Valid final JobSubmission jobSubmission
+    ) throws AgentLaunchException, IdAlreadyExistsException, SaveAttachmentException {
         final long start = System.nanoTime();
         final Set<Tag> tags = Sets.newHashSet();
         try {
@@ -91,14 +95,7 @@ public String launchJob(@Valid final JobSubmission jobSubmission) throws AgentLa
              * 5. If the agent launch fails mark the job failed else return
              */
 
-            final String jobId;
-            try {
-                jobId = this.jobPersistenceService.saveJobSubmission(jobSubmission);
-            } catch (final Throwable t) {
-                // TODO: Really handle this error
-                log.error("TODO", t);
-                throw new AgentLaunchException(t);
-            }
+            final String jobId = this.jobPersistenceService.saveJobSubmission(jobSubmission);
 
             final ResolvedJob resolvedJob;
             try {

diff --git a/...e-web/src/test/groovy/com/netflix/genie/web/services/impl/JobLaunchServiceImplSpec.groovy b/...e-web/src/test/groovy/com/netflix/genie/web/services/impl/JobLaunchServiceImplSpec.groovy
@@ -23,6 +23,8 @@ import com.netflix.genie.web.data.services.JobPersistenceService
 import com.netflix.genie.web.dtos.JobSubmission
 import com.netflix.genie.web.dtos.ResolvedJob
 import com.netflix.genie.web.exceptions.checked.AgentLaunchException
+import com.netflix.genie.web.exceptions.checked.IdAlreadyExistsException
+import com.netflix.genie.web.exceptions.checked.SaveAttachmentException
 import com.netflix.genie.web.services.JobResolverService
 import io.micrometer.core.instrument.simple.SimpleMeterRegistry
 import spock.lang.Specification
@@ -77,7 +79,31 @@ class JobLaunchServiceImplSpec extends Specification {
         0 * jobResolverService.resolveJob(_ as String)
         0 * jobPersistenceService.updateJobStatus(jobId, JobStatus.RESOLVED, JobStatus.ACCEPTED, _ as String)
         0 * agentLauncher.launchAgent(_ as ResolvedJob)
-        thrown(AgentLaunchException)
+        thrown(IllegalStateException)
+
+        when:
+        service.launchJob(jobSubmission)
+
+        then:
+        1 * jobPersistenceService.saveJobSubmission(jobSubmission) >> {
+            throw new IdAlreadyExistsException("try again")
+        }
+        0 * jobResolverService.resolveJob(_ as String)
+        0 * jobPersistenceService.updateJobStatus(jobId, JobStatus.RESOLVED, JobStatus.ACCEPTED, _ as String)
+        0 * agentLauncher.launchAgent(_ as ResolvedJob)
+        thrown(IdAlreadyExistsException)
+
+        when:
+        service.launchJob(jobSubmission)
+
+        then:
+        1 * jobPersistenceService.saveJobSubmission(jobSubmission) >> {
+            throw new SaveAttachmentException("hmm that's not good")
+        }
+        0 * jobResolverService.resolveJob(_ as String)
+        0 * jobPersistenceService.updateJobStatus(jobId, JobStatus.RESOLVED, JobStatus.ACCEPTED, _ as String)
+        0 * agentLauncher.launchAgent(_ as ResolvedJob)
+        thrown(SaveAttachmentException)
 
         when:
         service.launchJob(jobSubmission)