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

fix: only exclude actual 'run once' jobs from scheduler main list [DHIS2-16074] #15494

Merged
merged 1 commit into from
Oct 26, 2023

Conversation

jbee
Copy link
Contributor

@jbee jbee commented Oct 26, 2023

Summary

The filter applied to the scheduler main list was too unspecific and would also filter out disabled jobs that were removed from a queue and therefore would not have a CRON expression or delay time.

Manual Testing

  • create some jobs
  • create a queue with the new jobs
  • delete the queue
  • check the jobs get listed for /api/scheduler as disabled

@jbee jbee self-assigned this Oct 26, 2023
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

Comment on lines +344 to +347
return schedulingType == SchedulingType.ONCE_ASAP
&& cronExpression == null
&& delay == null
&& queueName == null;
Copy link
Contributor Author

@jbee jbee Oct 26, 2023

Choose a reason for hiding this comment

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

FYI: the reason to check all the fields is so we do not exclude scheduled jobs which got executed manually so their schedulingType shortly switches to ONCE_ASAP as well but they still have a cronExpression.

@codecov
Copy link

codecov bot commented Oct 26, 2023

Codecov Report

Merging #15494 (19fb26a) into master (51fb710) will increase coverage by 16.67%.
Report is 4 commits behind head on master.
The diff coverage is 0.00%.

@@              Coverage Diff              @@
##             master   #15494       +/-   ##
=============================================
+ Coverage     49.48%   66.15%   +16.67%     
- Complexity    23291    31188     +7897     
=============================================
  Files          3489     3487        -2     
  Lines        129699   129761       +62     
  Branches      15136    15136               
=============================================
+ Hits          64176    85842    +21666     
+ Misses        59448    36834    -22614     
- Partials       6075     7085     +1010     
Flag Coverage Δ
integration 49.71% <0.00%> (?)
integration-h2 32.40% <0.00%> (-0.01%) ⬇️
unit 30.33% <0.00%> (-0.04%) ⬇️

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

Files Coverage Δ
...ava/org/hisp/dhis/scheduling/JobConfiguration.java 76.11% <0.00%> (+2.98%) ⬆️

... and 1236 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

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

@jbee jbee enabled auto-merge (squash) October 26, 2023 11:11
@jbee jbee merged commit 220774a into dhis2:master Oct 26, 2023
18 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