-
Notifications
You must be signed in to change notification settings - Fork 1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Cancel job functionality #96
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,7 @@ | ||
package spark.jobserver | ||
|
||
import com.typesafe.config.ConfigFactory | ||
import spark.jobserver.JobManagerActor.KillJob | ||
import scala.collection.mutable | ||
import spark.jobserver.io.JobDAO | ||
|
||
|
@@ -195,5 +196,20 @@ abstract class JobManagerSpec extends JobSpecBase(JobManagerSpec.getNewSystem) { | |
case JobResult(_, result: Int) => result should equal (1 + 2 + 3) | ||
} | ||
} | ||
|
||
it("should be able to cancel running job") { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you add a test for what happens when the job is not running? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It will just call sparkContext.cancelJobGroup(NONEXISTING_GROUP_ID) which is safe (will not raise any exception) and will do actually nothing. In WebAPI it will actually handle this: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes, I'm saying there's no test for that scenario, the user should receive a message that no such job exists right? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. also, if you try to delete a job that already finished what would be the result? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @zeitos It seems from the code that deleting a nonexistant job would get the user the NotFound message. I think the same thing if the job already finished. |
||
manager ! JobManagerActor.Initialize | ||
expectMsgClass(classOf[JobManagerActor.Initialized]) | ||
|
||
uploadTestJar() | ||
manager ! JobManagerActor.StartJob("demo", classPrefix + "LongPiJob", stringConfig, allEvents) | ||
expectMsgPF(1 seconds, "Did not get JobResult") { | ||
case JobStarted(id, _, _) => { | ||
manager ! KillJob(id) | ||
expectMsgClass(classOf[JobKilled]) | ||
} | ||
} | ||
} | ||
|
||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: It stops the job with the specified jobId right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, jobserver job id (that returned after job submitted to jobserver).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes but, what I'm trying to say is that the documentation says "current job" , there might be several jobs currently working, you just want to cancel one.