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

Plat 249 reduce rerun time #1165

Merged
merged 17 commits into from
Nov 3, 2022
Merged

Plat 249 reduce rerun time #1165

merged 17 commits into from
Nov 3, 2022

Conversation

bradsawadye
Copy link
Contributor

No description provided.

This adds logic for returning the total number of transactions to be rerun when a bulk rerun is being done

PLAT-249
This is a refactor of the logic to reduce the time teken for a transaction to be rerun. Some redundant steps have been removed

PLAT-249
This is a more efficient way of creating the bulk rerun tasks. Before the openhim console had to fetch transcation ids and then send them back to the openhim core for the creation of rerrun tasks. The openhim console only has to specify the query parameters for the transactions to be rerun and the core will handle the creation of the tasks.
@codecov
Copy link

codecov bot commented Sep 9, 2022

Codecov Report

Merging #1165 (0236a38) into master (999047c) will increase coverage by 0.00%.
The diff coverage is 90.47%.

@@           Coverage Diff           @@
##           master    #1165   +/-   ##
=======================================
  Coverage   85.02%   85.02%           
=======================================
  Files          77       77           
  Lines        5247     5310   +63     
=======================================
+ Hits         4461     4515   +54     
- Misses        786      795    +9     
Impacted Files Coverage Δ
src/alerts.js 85.71% <ø> (ø)
src/tasks.js 79.34% <88.88%> (-4.28%) ⬇️
src/api/transactions.js 90.47% <90.69%> (+2.97%) ⬆️
src/koaApi.js 100.00% <100.00%> (ø)
src/middleware/messageStore.js 89.60% <0.00%> (-0.81%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@MattyJ007
Copy link
Contributor

Failed build = 🍰

Copy link
Contributor

@michaelloosen michaelloosen left a comment

Choose a reason for hiding this comment

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

Looks good, minor fixes

config/default.json Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
src/api/transactions.js Outdated Show resolved Hide resolved
src/api/transactions.js Outdated Show resolved Hide resolved
src/tasks.js Outdated Show resolved Hide resolved
src/tasks.js Outdated Show resolved Hide resolved
test/unit/tasksTest.js Outdated Show resolved Hide resolved
test/integration/transactionsAPITests.js Outdated Show resolved Hide resolved
src/api/transactions.js Show resolved Hide resolved
The projection was incorrect and causing some tests to fail
Should be version 7.1.0 not 7.2.0
This is so we dont have memory issues when a lot of transactions are being rerun
Make the active number of tasks processed concurrently configurable
To increase the code coverage
This is so we dont overflow the stack when there are lot of transactions being rerun.
Copy link
Contributor

@marrouchi marrouchi left a comment

Choose a reason for hiding this comment

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

I hope the review is beneficial 🙏

src/alerts.js Show resolved Hide resolved
test/unit/tasksTest.js Outdated Show resolved Hide resolved
test/unit/transactionsTest.js Show resolved Hide resolved
src/tasks.js Show resolved Hide resolved
Co-authored-by: Mohamed Marrouchi <marrouchi.mohamed@gmail.com>
Copy link
Contributor

@michaelloosen michaelloosen left a comment

Choose a reason for hiding this comment

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

LGTM

@michaelloosen michaelloosen merged commit 5ece1a1 into master Nov 3, 2022
@michaelloosen michaelloosen deleted the PLAT-249-reduce-rerun-time branch November 3, 2022 08:54
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