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

chore: Make async tracker importer use scheduler #15597

Merged
merged 21 commits into from
Nov 14, 2023

Conversation

enricocolasante
Copy link
Contributor

@enricocolasante enricocolasante commented Nov 3, 2023

Created TrackerJobImport for async calls of Tracker Importer.
TrackerImportController endpoints are converting the incoming request to a Body object that adds some logic to be able to process the import. In particular tracker importer allows the client to send entities to be created without a UID and the BodyConverter creates all missing UIDs in the payload.
So TrackerObjects is the object that is serialized/deserialized and saved as a resource to be used by the Scheduler

The big amount of changed files is caused by the change of some key signatures in the importer and this was necessary because in the previous implementation there was no good distinction between data and parameters.

Other changes:

  • Removing timingStats from ImportReport, they are going to be present in JobProgress

@enricocolasante enricocolasante force-pushed the import_tracker_scheduler branch from 8385298 to a3df880 Compare November 3, 2023 14:40
@enricocolasante enricocolasante force-pushed the import_tracker_scheduler branch from a3df880 to bb58ec4 Compare November 3, 2023 15:14
Copy link

codecov bot commented Nov 7, 2023

Codecov Report

Merging #15597 (9908810) into master (ae674bb) will decrease coverage by 0.04%.
Report is 2 commits behind head on master.
The diff coverage is 93.63%.

@@             Coverage Diff              @@
##             master   #15597      +/-   ##
============================================
- Coverage     66.20%   66.17%   -0.04%     
+ Complexity    31264    31231      -33     
============================================
  Files          3487     3482       -5     
  Lines        129902   129815      -87     
  Branches      15170    15170              
============================================
- Hits          86007    85901     -106     
- Misses        36813    36837      +24     
+ Partials       7082     7077       -5     
Flag Coverage Δ
integration 49.88% <80.89%> (+0.10%) ⬆️
integration-h2 32.28% <49.68%> (-0.13%) ⬇️
unit 30.30% <66.24%> (-0.03%) ⬇️

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

Files Coverage Δ
...his/scheduling/HibernateJobConfigurationStore.java 38.09% <ø> (ø)
...org/hisp/dhis/tracker/imports/ParamsConverter.java 93.33% <100.00%> (ø)
...isp/dhis/tracker/imports/TrackerIdSchemeParam.java 90.90% <ø> (ø)
...sp/dhis/tracker/imports/TrackerIdSchemeParams.java 100.00% <ø> (ø)
...is/tracker/imports/TrackerIdentifierCollector.java 96.73% <100.00%> (ø)
...isp/dhis/tracker/imports/TrackerImportService.java 100.00% <100.00%> (ø)
...er/imports/bundle/DefaultTrackerBundleService.java 94.59% <100.00%> (+5.70%) ⬆️
...g/hisp/dhis/tracker/imports/domain/Coordinate.java 0.00% <ø> (ø)
...g/hisp/dhis/tracker/imports/domain/Enrollment.java 100.00% <ø> (ø)
...va/org/hisp/dhis/tracker/imports/domain/Event.java 75.00% <ø> (ø)
... and 34 more

... and 18 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 dae3308...9908810. Read the comment docs.

@enricocolasante enricocolasante marked this pull request as ready for review November 10, 2023 08:26
@david-mackessy
Copy link
Contributor

david-mackessy commented Nov 10, 2023

Should there be an actual TrackerJob of some sort that implements the org.hisp.dhis.scheduling.Job interface somewhere? AFAIK that needs to be done for the JobScheduler to pick them up and call execute on.

@enricocolasante
Copy link
Contributor Author

Should there be an actual TrackerJob of some sort that implements the org.hisp.dhis.scheduling.Job interface somewhere? AFAIK that needs to be done for the JobScheduler to pick them up and call execute on.

Exactly and this is done in org.hisp.dhis.webapi.controller.tracker.imports.TrackerImportJob

@david-mackessy
Copy link
Contributor

david-mackessy commented Nov 10, 2023

Should there be an actual TrackerJob of some sort that implements the org.hisp.dhis.scheduling.Job interface somewhere? AFAIK that needs to be done for the JobScheduler to pick them up and call execute on.

Exactly and this is done in org.hisp.dhis.webapi.controller.tracker.imports.TrackerImportJob

Ah thanks Enrico :) I just did a simple search for 'implements Job 'in the PR but that class code wasn't expanded because of the size of the diff.

Copy link
Contributor

@david-mackessy david-mackessy left a comment

Choose a reason for hiding this comment

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

nice job and it's covered with e2e tests. Great.

Copy link

SonarCloud Quality Gate failed.    Quality Gate failed

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

0.0% 0.0% Coverage
0.4% 0.4% Duplication

idea Catch issues before they fail your Quality Gate with our IDE extension sonarlint SonarLint

@enricocolasante enricocolasante merged commit 5cd719c into master Nov 14, 2023
14 of 15 checks passed
@enricocolasante enricocolasante deleted the import_tracker_scheduler branch November 14, 2023 10:09
enricocolasante added a commit that referenced this pull request Nov 14, 2023
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