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

feat: save file resource content synchronously for jobs [DHIS2-15276] #15747

Merged
merged 1 commit into from
Nov 21, 2023

Conversation

jbee
Copy link
Contributor

@jbee jbee commented Nov 21, 2023

Summary

To ensure the file content of a job that runs immediately is already saved a new method is added to the file resources service which stores the content synchronously via JClouds.

For clarity the existing methods that save the content asynchronously were renamed to reflect this.

New error codes are added for two cases:

  • failed to store content synchronously
  • content was loaded expecting it exists but it still is null

Manual Testing

  • run many metadata and/or tracker imports (do many POSTs in short order, could be same payload)
  • check they all execute in a sequence with no multiple-seconds gaps (see timestamps of the created job configurations)

Automatic Testing

Existing tests indirectly cover the synchronous content storage as it is now used for metadata and tracker imports.
If all of them pass and the runtime for e2e tests has not increased notably beyond about 15min the behavior is as expected.

The new two error codes cannot be integration tested as this requires a failure of the cloud storage which cannot be simulated (with reasonable effort) except via mocking which at this point adds little value.

@jbee jbee self-assigned this Nov 21, 2023
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

Copy link

codecov bot commented Nov 21, 2023

Codecov Report

Merging #15747 (25bae50) into master (a0262b5) will not change coverage.
Report is 3 commits behind head on master.
The diff coverage is 76.92%.

Additional details and impacted files
@@            Coverage Diff            @@
##             master   #15747   +/-   ##
=========================================
  Coverage     66.29%   66.29%           
+ Complexity    31386    31383    -3     
=========================================
  Files          3486     3486           
  Lines        129991   129991           
  Branches      15201    15201           
=========================================
  Hits          86184    86184           
+ Misses        36716    36714    -2     
- Partials       7091     7093    +2     
Flag Coverage Δ
integration 50.07% <76.92%> (-0.01%) ⬇️
integration-h2 32.33% <23.07%> (+<0.01%) ⬆️
unit 30.27% <15.38%> (+<0.01%) ⬆️

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

Files Coverage Δ
...rc/main/java/org/hisp/dhis/feedback/ErrorCode.java 100.00% <100.00%> (ø)
...his/scheduling/DefaultJobConfigurationService.java 70.96% <100.00%> (ø)
.../org/hisp/dhis/webapi/utils/FileResourceUtils.java 76.04% <100.00%> (ø)
.../dhis/pushanalysis/DefaultPushAnalysisService.java 1.68% <0.00%> (ø)
.../dhis/fileresource/DefaultFileResourceService.java 56.77% <75.00%> (+1.77%) ⬆️

... and 4 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 48c63cf...25bae50. Read the comment docs.

@jbee jbee enabled auto-merge (squash) November 21, 2023 12:48
@jbee jbee merged commit f5161c5 into dhis2:master Nov 21, 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