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: Cohort Replicator JS #78

Merged
merged 13 commits into from
Sep 26, 2023
Merged

Conversation

fmarek-kindred
Copy link
Contributor

@fmarek-kindred fmarek-kindred commented Sep 12, 2023

Once we merge that one, as separate work, I will finish renaming of cohort packages.

There are many files in this PR but mostly because of import change and moving classes around.

while (attemptNr <= retry.maxAttempts) {
attemptNr++

if (attemptNr > retry.maxAttempts) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This one is missing in Replicator Rust example!

@codecov
Copy link

codecov bot commented Sep 12, 2023

Codecov Report

Patch coverage: 8.51% and project coverage change: -1.99% ⚠️

Comparison is base (29cee52) 60.15% compared to head (8d9180b) 58.16%.
Report is 1 commits behind head on master.

❗ Current head 8d9180b differs from pull request most recent head 4cecb9f. Consider uploading reports for the commit 4cecb9f to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master      #78      +/-   ##
==========================================
- Coverage   60.15%   58.16%   -1.99%     
==========================================
  Files          96      103       +7     
  Lines        5132     5314     +182     
==========================================
+ Hits         3087     3091       +4     
- Misses       2045     2223     +178     
Files Changed Coverage Δ
...nking_common/src/state/postgres/database_config.rs 77.27% <0.00%> (ø)
packages/banking_replicator/src/app.rs 0.00% <0.00%> (ø)
...kages/banking_replicator/src/statemap_installer.rs 0.00% <ø> (ø)
packages/cohort_banking/src/app.rs 0.00% <ø> (ø)
.../src/callbacks/certification_candidate_provider.rs 0.00% <ø> (ø)
...kages/cohort_banking/src/callbacks/oo_installer.rs 0.00% <ø> (ø)
packages/cohort_banking/src/lib.rs 100.00% <ø> (ø)
packages/cohort_banking/src/model/requests.rs 0.00% <ø> (-57.90%) ⬇️
packages/cohort_sdk/src/cohort.rs 0.00% <ø> (ø)
packages/cohort_sdk/src/model/mod.rs 0.00% <ø> (ø)
... and 10 more

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

fmarek-kindred and others added 6 commits September 13, 2023 14:02
* feat: Initial draft for errors handling between Rust and JS

* fix: Remove cohort_sdk_js dependency, instead declare optional dependencies on images.

* fix: Inject using serde annotation _typ during serialisation into JSON.

* fix: Improve error handling when processig callback calls between JS and RS.

* feat: Add ReplicatorError and map it to JS layer. (#81)

* feat: Add wrapper and error handler for Replicator JS.

* feat: Add ReplicatorError and map it to JS layer.

* fix: Move dev dependencies into relevant section in the package.json
let database = Database::init_db(cfg_db).await.map_err(|e| e.to_string()).unwrap();
let database = Database::init_db(cfg_db.clone())
.await
.map_err(|e| e.to_string())
Copy link
Collaborator

Choose a reason for hiding this comment

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

don't need this map_err as you are chaining that with expect

@fmarek-kindred fmarek-kindred merged commit d99010a into master Sep 26, 2023
2 checks passed
@fmarek-kindred fmarek-kindred deleted the feature/cohort_replicator_js branch September 26, 2023 00:10
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.

2 participants