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

run user-defined functionality in job_started and job_completed hooks #649

Merged
merged 11 commits into from
Jul 19, 2024

Conversation

evawoodbridge
Copy link
Contributor

@evawoodbridge evawoodbridge commented Jul 17, 2024

Adds the ability to inject bash into the job started and job completed hook through the runner group in the runner-manager.yaml config

Resolves issue raised: #577

@evawoodbridge evawoodbridge requested a review from a team as a code owner July 17, 2024 11:20
@evawoodbridge evawoodbridge marked this pull request as draft July 17, 2024 13:21
@evawoodbridge evawoodbridge marked this pull request as ready for review July 17, 2024 13:31
Copy link
Contributor

@tcarmet tcarmet left a comment

Choose a reason for hiding this comment

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

This is looking pretty good! Thank you for this valuable contribution!

If possible and if it's not too much to ask, I would love to see some basic templating test, just like it is done in the base backend tests. In the same spirit of what you did in the runner_group test, just to ensure the templating is behaving as expected,

Let me know what you think and thanks again.

@codecov-commenter
Copy link

codecov-commenter commented Jul 17, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.76%. Comparing base (77cdd70) to head (9c2f97a).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #649      +/-   ##
==========================================
+ Coverage   86.68%   86.76%   +0.07%     
==========================================
  Files          34       34              
  Lines        1412     1420       +8     
==========================================
+ Hits         1224     1232       +8     
  Misses        188      188              
Flag Coverage Δ
api 65.14% <100.00%> (+0.19%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@evawoodbridge
Copy link
Contributor Author

I have added the test, very open to feedback or changes

Copy link
Contributor

@tcarmet tcarmet left a comment

Choose a reason for hiding this comment

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

Thanks for adding the extra test, made a small suggestion to try to make it work and we should be good to go 🚀

Have a look at the formatting correction needed as well. If you have trunk installed, simply running trunk fmt should do the trick. If you don't have it installed, you can use the codespace provided with the repo.

tests/unit/backend/test_base.py Outdated Show resolved Hide resolved
@tcarmet tcarmet merged commit 99a4199 into scality:main Jul 19, 2024
9 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants