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.

Sign up for GitHub

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

Jump to bottom

Handle transmission mode reference #419

Merged
merged 43 commits into from
Mar 31, 2022
Merged

Handle transmission mode reference #419

merged 43 commits into from
Mar 31, 2022

Conversation

fmigneault
Copy link
Collaborator

@fmigneault fmigneault commented Mar 17, 2022 •
edited
Loading

Changes

Many adjustments for OGC compliance.

  • Everything from first implementation prefer header for execute job #415
  • Properly return the results directly when running in sync mode (instead of status info)
  • Generate the Link header for outputs requested with transmissionMode=reference
  • Some jobs utils refactors to align with new supported features

References

@fmigneault
first implementation prefer header for execute job
72ba589
@fmigneault
support Prefer header properly (resolves #247) + allow omitting outpu…
6de71bc 
…ts in execute (fix #375)
@fmigneault
adjust conformance prefer return none (relates to #414)
161cf9c
@fmigneault
Merge branch 'master' into job-prefer-header
bf02f08
@fmigneault
remove references to fix #247 in CLI, always using async mode with mo…
5520872 
…nitoring to obtain sync status
@fmigneault
remove reference to fix #375 in CLI, outputs can be omitted but not f…
bed33da 
…iltered (#380)
@fmigneault
fix test to consider job exec params now optional to allow alternatives
42640cd
@fmigneault
fix lint
637adca
@fmigneault
celery beat to worker docker command for task result cleaup
7b938fc
@fmigneault
ignore linkcheck celery docs down (relates to https://github.com/cele…
b59949f 
…ry/celery/issues/7351)
@fmigneault
fix lint
ff2f94c
@fmigneault
ensure jobControlOptions and outputTransmission are reported sorted
9d56a4e
@fmigneault
replace all outputTransmission 'reference' by 'value' to correctly re…
25fdd0e 
…port default Weaver behaviour
@fmigneault
Merge branch 'master' into job-prefer-header
83adf03
@fmigneault
implement Link header creation for transmissionMode=reference (relate…
b51bbfe 
…s to #377)
@fmigneault fmigneault self-assigned this Mar 17, 2022
@github-actions github-actions bot added ci/doc Issue related to documentation of the package ci/operations Related to CI operations (actions, execution, install, builds, etc.) ci/tests Tests of the package and features feature/docker Issue related to Docker application package execution. process/builtin Issue related to builtin application processes labels Mar 17, 2022
@github-actions github-actions bot added feature/cli Issues or features related to CLI operations. 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. feature/opensearch Issue related to OpenSearch functionalities. feature/providers Issue related to providers convertion to WPS-REST processes. labels Mar 17, 2022
@fmigneault
revert weaver default jobControlOption async only - if sync needed, m…
b70cb9c 
…ust be provided at deploy time
@fmigneault
revert test for supported jobControlOptions
0e9119a
@fmigneault
ignore except too broad for int check
170c6fd
@fmigneault
fix doc indents
2ba0679
@fmigneault
quote requests typing
3ac2830
@fmigneault
Merge branch 'job-prefer-header' into output-ref
abc640f
@fmigneault
add ref execute schema not required inputs
bf99d10
@fmigneault
Merge branch 'job-prefer-header' into output-ref
51050bc
@fmigneault
fix lint
c81a99c
@fmigneault
adjust sync result returned directly + support response=raw for singl…
fd0e83e 
…e outputs or multi-by-ref (relates to #376)
@fmigneault
update docs with new response=raw, mode=sync and transmissionMode=val…
68ba237 
…ue/reference supports (relates to #376, #377)
@fmigneault
fix imports linting
b58fb4a
@fmigneault
fix tests handling dispatched WPS request monitoring
dfc4f76
@fmigneault
fix dismiss mock
d7330fb
@fmigneault
fix test for response=raw/transmissionMode=reference + document job i…
50ff933 
…nputs endpoint
@fmigneault
fix doc refs
4d9b43a
@fmigneault
add CLI proper support of --ref for transmissionMode=reference and --…
b1a0883 
…download option of those file links
@fmigneault
fix msg when listing results directly
c7295ae
@fmigneault fmigneault added the project/OGC Related to OGC testbeds or relavant projects. label Mar 23, 2022
@fmigneault fmigneault marked this pull request as ready for review March 24, 2022 00:27
@fmigneault fmigneault requested a review from dbyrns March 24, 2022 00:27
@fmigneault fmigneault mentioned this pull request Mar 24, 2022
Merged
dbyrns
dbyrns reviewed Mar 30, 2022
View reviewed changes
weaver/wps_restapi/jobs/utils.py
if job.status in JOB_STATUS_CATEGORIES[StatusCategory.RUNNING]:
# signal to stop celery task. Up to it to terminate remote if any.
LOGGER.debug("Job [%s] dismiss operation: Canceling task [%s]", job.id, job.task_id)
celery_app.control.revoke(job.task_id, terminate=True)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that the revoke command is asynchronous, could the cleanup following it (removing logs, xml, dir) be done prematurily while the process is still spitting logs waiting to be killed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is a possibility. I haven't explored this much.
If we encounter this, we could think of a way to handle it. Probably need to run another task with a delay to do a cleanup with below checks.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In fact, the succeeded jobs also leave their stuff, so a mechanism to clean that should already be in place anyway. For the typical birds dumping the result in the output directory, there was a cron job for that. Does it apply to Weaver also?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The results that were generated by pywps and cwltool should be all cleaned up following execution by weaver after it moved them to wps-outputs. There should be 3 items {jobId}.log, {jobId}.xml (WPS status), {jobId}/ (output results).

The dismiss cleanup applied here is more specific for OGC Dismiss operation that requests explicitly a cleanup of those outputs, given a user downloaded them.

If the cron job runs inside wps-outputs/ for each birds, then it should pick up weaver as well, but there is not a cron specifically within weaver.

dbyrns
dbyrns approved these changes Mar 30, 2022
View reviewed changes
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.

Goog PR. There is still an open comment, but it's not a deal breaker.

@fmigneault fmigneault merged commit 9ca6bbe into master Mar 31, 2022
@fmigneault fmigneault deleted the output-ref branch March 31, 2022 13:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dbyrns dbyrns dbyrns approved these changes

Assignees

@fmigneault fmigneault

Labels
ci/doc Issue related to documentation of the package ci/operations Related to CI operations (actions, execution, install, builds, etc.) ci/tests Tests of the package and features feature/cli Issues or features related to CLI operations. feature/db Related to database or datatype manipulation. feature/docker Issue related to Docker application package execution. feature/job Issues related to job execution, reporting and logging. feature/oas Issues related to OpenAPI specifications. feature/opensearch Issue related to OpenSearch functionalities. feature/providers Issue related to providers convertion to WPS-REST processes. process/builtin Issue related to builtin application processes process/wps3 Issue related to WPS 3.x (REST-JSON) processes support project/OGC Related to OGC testbeds or relavant projects.
Projects
None yet
Milestone
No milestone
2 participants
@fmigneault @dbyrns