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

Draft: re-added notify_on #2020

Merged
merged 2 commits into from
Dec 16, 2024
Merged

Draft: re-added notify_on #2020

merged 2 commits into from
Dec 16, 2024

Conversation

kinow
Copy link
Member

@kinow kinow commented Dec 13, 2024

In GitLab by @dbeltrankyl on Dec 10, 2024, 14:55

This adds again the notify_on attribute.

JOB:
 NOTIFY_ON:
   - RUNNING
   - COMPLETED

Also the

mail:

 to: 
   - mail_1

expects a yaml_list

#1484 , #1468

TODO:

  • test in bschubs when working, or any other automail.
  • docs
  • add pytests if confirmed this is enough

I believe the automail of BSC can't be tested outside the ibsc

@kinow kinow added this to the Autosubmit 4.1.12 milestone Dec 13, 2024
@kinow
Copy link
Member Author

kinow commented Dec 13, 2024

In GitLab by @dbeltrankyl on Dec 10, 2024, 14:57

added 2 commits

Compare with previous version

@kinow
Copy link
Member Author

kinow commented Dec 13, 2024

In GitLab by @dbeltrankyl on Dec 10, 2024, 15:07

marked this merge request as draft

@kinow
Copy link
Member Author

kinow commented Dec 13, 2024

In GitLab by @dbeltrankyl on Dec 10, 2024, 15:55

Tested on the bschubs

Autosubmit notification
-------------------------

Experiment id:  a826 

Job name: a826_LOCAL_SETUP 

The job status has changed from: SUBMITTED to COMPLETED 




INFO: This message was auto generated by Autosubmit, remember that you can disable these messages on Autosubmit config file. 

Mail recieved

@kinow
Copy link
Member Author

kinow commented Dec 13, 2024

In GitLab by @dbeltrankyl on Dec 10, 2024, 15:57

marked the checklist item test in bschubs when working, or any other automail. as completed

@kinow
Copy link
Member Author

kinow commented Dec 13, 2024

In GitLab by @dbeltrankyl on Dec 11, 2024, 15:32

Requires BSC-ES/autosubmit-config-parser#58 ( and update autosubmitconfigparser)

@kinow
Copy link
Member Author

kinow commented Dec 13, 2024

In GitLab by @dbeltrankyl on Dec 11, 2024, 15:32

marked the checklist item add pytests if confirmed this is enough as completed

@kinow
Copy link
Member Author

kinow commented Dec 13, 2024

In GitLab by @dbeltrankyl on Dec 11, 2024, 15:47

added 1 commit

  • 5888664 - Added test to check attr value ( notify_on for now)

Compare with previous version

@kinow
Copy link
Member Author

kinow commented Dec 13, 2024

In GitLab by @dbeltrankyl on Dec 11, 2024, 15:50

added 1 commit

Compare with previous version

@kinow
Copy link
Member Author

kinow commented Dec 13, 2024

In GitLab by @dbeltrankyl on Dec 11, 2024, 15:51

Ready to review

  • Added a pytest in autosubmit code to check the value of the job.notify_on attribute ( made it general to add all the attributes eventually)
  • Updated docs
  • Updated as configparser version # the pipeline won't work until notify_on normalized autosubmit-config-parser#58 is merged

@kinow
Copy link
Member Author

kinow commented Dec 13, 2024

In GitLab by @dbeltrankyl on Dec 11, 2024, 15:51

@isimo00 you can review this one if you want ( also the configparser associated one )

@kinow
Copy link
Member Author

kinow commented Dec 13, 2024

In GitLab by @dbeltrankyl on Dec 11, 2024, 15:52

requested review from @isimo00

@kinow
Copy link
Member Author

kinow commented Dec 13, 2024

In GitLab by @dbeltrankyl on Dec 11, 2024, 15:54

There is also a change in the mail_to code.. but it is a in-line comment . We can do a similar merge for that variable. I think there are some issues already created for the mail_to #1472 #1435( not sure about this one).

I've updated the mail_to doc to reflect that it expects a list right now

@kinow
Copy link
Member Author

kinow commented Dec 13, 2024

In GitLab by @dbeltrankyl on Dec 11, 2024, 15:55

added 1 commit

Compare with previous version

@kinow
Copy link
Member Author

kinow commented Dec 13, 2024

In GitLab by @dbeltrankyl on Dec 11, 2024, 15:56

marked the checklist item docs as completed

@kinow
Copy link
Member Author

kinow commented Dec 13, 2024

In GitLab by @isimo00 on Dec 11, 2024, 16:11

All seems good, thank you @dbeltrankyl !

@kinow
Copy link
Member Author

kinow commented Dec 13, 2024

In GitLab by @isimo00 on Dec 11, 2024, 16:12

approved this merge request

@kinow
Copy link
Member Author

kinow commented Dec 13, 2024

In GitLab by @isimo00 on Dec 11, 2024, 16:14

Well, I see the pipeline failed but I see it's because of the configparser version requirement so it's all good. I assume you ran it locally and everything passed

@isimo00 isimo00 self-requested a review December 16, 2024 10:27
@dbeltrankyl dbeltrankyl force-pushed the gl-1468,1484-notify-on branch from 066bc39 to 883ccad Compare December 16, 2024 14:13
@dbeltrankyl dbeltrankyl force-pushed the gl-1468,1484-notify-on branch from 883ccad to 3cedc38 Compare December 16, 2024 14:18
@dbeltrankyl
Copy link
Contributor

Rebased and squashed, waiting for the pipeline...

@dbeltrankyl
Copy link
Contributor

dbeltrankyl commented Dec 16, 2024

I did the rebase wrongly and corrected it,.. running the pipeline...

@dbeltrankyl
Copy link
Contributor

Success, merging

@dbeltrankyl dbeltrankyl merged commit 156b6e7 into master Dec 16, 2024
8 checks passed
@dbeltrankyl dbeltrankyl deleted the gl-1468,1484-notify-on branch December 16, 2024 14:32
@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 88.88889% with 1 line in your changes missing coverage. Please review.

Project coverage is 48.93%. Comparing base (1f5ded1) to head (a778714).
Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
autosubmit/notifications/mail_notifier.py 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2020      +/-   ##
==========================================
+ Coverage   48.90%   48.93%   +0.02%     
==========================================
  Files          72       72              
  Lines       17517    17525       +8     
  Branches     3408     3408              
==========================================
+ Hits         8567     8575       +8     
  Misses       8150     8150              
  Partials      800      800              
Flag Coverage Δ
fast-tests 48.93% <88.88%> (+0.02%) ⬆️

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.

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.

4 participants