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

support callback request subscribers and job notification refactoring #572

Merged
merged 17 commits into from
Oct 13, 2023

Conversation

fmigneault
Copy link
Collaborator

@fmigneault fmigneault commented Oct 7, 2023

Changes

  • Add Job subscribers support to define OGC-compliant callback URLs where HTTP(S) requests will be sent upon
    reaching certain Job status milestones (fixes [Feature] Support and validate Job notification callback URL #230).
  • Add email notification support to the new subscribers definition (extension over OGC minimal requirements).
  • Deprecate Job notification_email in the OpenAPI specification in favor of subscribers, but preserve
    parsing of its value if provided in the JSON body during Job submission for backward compatibility support of
    existing servers. The Job.notification_email attribute is removed to avoid duplicate references.
  • Add notification email for Job started status, only available through the subscribers property.
  • Add {PROCESS_ID}/{STATUS}.mako template detection under the weaver.wps_email_notify_template_dir location
    to allow per-Process and per-Job status email customization.
  • Refactor weaver/notify.py and weaver/processes/execution.py to avoid mixed references to the
    encryption/decryption logic employed for notification emails. All notifications including emails and
    callback requests are now completely handled and contained in the weaver/notify.py module.
  • Remove partially duplicate Mako Template definition as hardcoded string and separate file for email notification.

References

@fmigneault fmigneault self-assigned this Oct 7, 2023
@fmigneault fmigneault added project/OGC Related to OGC testbeds or relavant projects. feature/job Issues related to job execution, reporting and logging. labels Oct 7, 2023
@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 7, 2023
@codecov
Copy link

codecov bot commented Oct 7, 2023

Codecov Report

Merging #572 (fe55014) into master (1572fd0) will increase coverage by 0.16%.
The diff coverage is 95.48%.

@@            Coverage Diff             @@
##           master     #572      +/-   ##
==========================================
+ Coverage   84.71%   84.87%   +0.16%     
==========================================
  Files          79       79              
  Lines       17828    17910      +82     
  Branches     2708     2716       +8     
==========================================
+ Hits        15103    15202      +99     
+ Misses       1992     1980      -12     
+ Partials      733      728       -5     
Files Coverage Δ
weaver/notify.py 92.19% <100.00%> (+11.55%) ⬆️
weaver/store/base.py 100.00% <ø> (ø)
weaver/store/mongodb.py 78.31% <ø> (ø)
weaver/wps_restapi/api.py 78.97% <ø> (ø)
weaver/wps_restapi/jobs/jobs.py 89.63% <100.00%> (ø)
weaver/wps_restapi/jobs/utils.py 77.94% <ø> (ø)
weaver/wps_restapi/swagger_definitions.py 99.82% <100.00%> (+<0.01%) ⬆️
weaver/processes/execution.py 86.28% <80.00%> (+1.95%) ⬆️
weaver/cli.py 86.30% <94.28%> (+0.39%) ⬆️
weaver/datatype.py 79.08% <62.50%> (ø)

@github-actions github-actions bot added the feature/cli Issues or features related to CLI operations. label Oct 10, 2023
perronld
perronld previously approved these changes Oct 11, 2023
Copy link
Contributor

@perronld perronld left a comment

Choose a reason for hiding this comment

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

Works well with Finch!
( using notification_email)

Copy link
Contributor

@ChaamC ChaamC left a comment

Choose a reason for hiding this comment

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

Minor fixes on outputs/comments/variable names, the rest looks good to me.
Good job!

weaver/cli.py Show resolved Hide resolved
weaver/cli.py Outdated Show resolved Hide resolved
weaver/cli.py Outdated Show resolved Hide resolved
weaver/notify.py Show resolved Hide resolved
weaver/notify.py Show resolved Hide resolved
tests/functional/test_cli.py Outdated Show resolved Hide resolved
Nazim-crim
Nazim-crim previously approved these changes Oct 12, 2023
Copy link
Contributor

@Nazim-crim Nazim-crim left a comment

Choose a reason for hiding this comment

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

LGTM, if I understood this correctly, multiple subscribers are only supported for different job status. Do you think it could be interesting to allow multiple emails notification for each status ?

@fmigneault
Copy link
Collaborator Author

LGTM, if I understood this correctly, multiple subscribers are only supported for different job status.

Yes

Do you think it could be interesting to allow multiple emails notification for each status ?

Could be, but I think we can work around it with a mailing list for the moment without changing the code.
If an actual use case to support multiple emails arises, it could be added later on.

@fmigneault fmigneault dismissed stale reviews from Nazim-crim and perronld via ef61f97 October 12, 2023 18:57
@fmigneault fmigneault merged commit d1cf5a9 into master Oct 13, 2023
43 checks passed
@fmigneault fmigneault deleted the subscribers branch October 13, 2023 18:34
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/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. process/wps3 Issue related to WPS 3.x (REST-JSON) processes support project/OGC Related to OGC testbeds or relavant projects.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature] Support and validate Job notification callback URL
4 participants