-
Notifications
You must be signed in to change notification settings - Fork 6
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
add more process/job links + add batch job dismiss operation #346
Conversation
Codecov Report
@@ Coverage Diff @@
## master #346 +/- ##
==========================================
+ Coverage 77.77% 77.89% +0.11%
==========================================
Files 63 63
Lines 9989 10119 +130
Branches 1511 1531 +20
==========================================
+ Hits 7769 7882 +113
- Misses 1734 1748 +14
- Partials 486 489 +3
Continue to review full report at Codecov.
|
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.
It looks good. See my little comments.
# PyWPS uses: [Accepted, Started, Succeeded, Failed, Paused, Exception] | ||
# OWSLib users: [Accepted, Running, Succeeded, Failed, Paused] (with 'Process' in front) | ||
# OGC compliant (old): [Accepted, Running, Succeeded, Failed] | ||
# OGC compliant (new): [accepted, running, successful, failed, dismissed] |
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.
It's sad that they do not version the revision. It would be great to know temporaly to which version we are compliant.
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.
OGC is still technically in v1.0-draft6 (since a few months already).
This should become the official v1.0 eventually (not sure what is the holdup).
The "old" in this case is ~ testbed-14/15.
The "new" is was is reported since, but I have seen instances using both variations of succeeded
and successful
.
Personally, I which they switched to succeeded
to be consistent with all other <state>ed
naming, but I'm sure we will have to live with it.
It will be a lot of fixes to switch to successful
.
All local, remote and E2E tests use succeeded
at the moment.
@@ -287,26 +373,33 @@ def get_job_status(request): | |||
return HTTPOk(json=job_status) | |||
|
|||
|
|||
def cancel_job_task(job, container): | |||
# type: (Job, AnySettingsContainer) -> Job | |||
app.control.revoke(job.task_id, terminate=True) # signal to stop celery task. Up to it to terminate remote if any. |
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.
Does EMS propagate the cancel signal to the underlying ADES job?
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.
No it doesn't for the moment. That is a good point.
This is one more item that should be addressed once a valid (more recent) Workflow is available from what @f-PLT was working on.
update
time to better track 'latest' job milestones(avoid checking many fields depending on status)