Skip to content
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

Merged
merged 10 commits into from
Oct 19, 2021

Conversation

fmigneault
Copy link
Collaborator

@fmigneault fmigneault commented Oct 15, 2021

@fmigneault fmigneault added triage/conformance Issue related to fixing/ensuring compliance to specifications. feature/job Issues related to job execution, reporting and logging. labels Oct 15, 2021
@fmigneault fmigneault self-assigned this Oct 15, 2021
@github-actions github-actions bot added ci/doc Issue related to documentation of the package ci/tests Tests of the package and features feature/db Related to database or datatype manipulation. feature/oas Issues related to OpenAPI specifications. process/wps3 Issue related to WPS 3.x (REST-JSON) processes support labels Oct 15, 2021
@codecov
Copy link

codecov bot commented Oct 15, 2021

Codecov Report

Merging #346 (9b41c69) into master (91736ca) will increase coverage by 0.11%.
The diff coverage is 82.10%.

Impacted file tree graph

@@            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     
Impacted Files Coverage Δ
weaver/utils.py 81.39% <ø> (ø)
weaver/store/mongodb.py 65.25% <40.00%> (+1.25%) ⬆️
weaver/datatype.py 69.91% <74.35%> (-0.23%) ⬇️
weaver/processes/execution.py 82.01% <77.77%> (-0.28%) ⬇️
weaver/wps_restapi/jobs/jobs.py 78.49% <79.56%> (+1.28%) ⬆️
weaver/exceptions.py 86.13% <100.00%> (+0.13%) ⬆️
weaver/status.py 93.87% <100.00%> (+0.39%) ⬆️
weaver/wps_restapi/api.py 77.01% <100.00%> (+0.13%) ⬆️
weaver/wps_restapi/processes/__init__.py 100.00% <100.00%> (ø)
weaver/wps_restapi/providers/__init__.py 100.00% <100.00%> (ø)
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 91736ca...9b41c69. Read the comment docs.

Copy link
Contributor

@dbyrns dbyrns left a 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]
Copy link
Contributor

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.

Copy link
Collaborator Author

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.
Copy link
Contributor

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?

Copy link
Collaborator Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci/doc Issue related to documentation of the package ci/tests Tests of the package and features feature/db Related to database or datatype manipulation. feature/job Issues related to job execution, reporting and logging. feature/oas Issues related to OpenAPI specifications. process/wps3 Issue related to WPS 3.x (REST-JSON) processes support triage/conformance Issue related to fixing/ensuring compliance to specifications.
Projects
None yet
2 participants