Skip to content

Conversation

Bravo555
Copy link
Member

Proposed changes

As in #2904, control flow was simplified by replacing fragmented control flow and hashmaps with tokio tasks and regular async/await.

A test was added to confirm that requests are still processed concurrently.

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Improvement (general improvements like code refactoring that doesn't explicitly fix a bug or add any new functionality)
  • Documentation Update (if none of the other choices apply)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Paste Link to the issue


Checklist

  • I have read the CONTRIBUTING doc
  • I have signed the CLA (in all commits with git commit -s)
  • I ran cargo fmt as mentioned in CODING_GUIDELINES
  • I used cargo clippy as mentioned in CODING_GUIDELINES
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

Further comments

Commits are kept small for ease of reviewing, once reviewed I'll merge them and add a description, unless it's not desired.

Fragmented uploads and downloads are only left in FileCacheActor and LogManagerActor, I will do them in next PRs.

Copy link

codecov bot commented Aug 27, 2024

Codecov Report

Attention: Patch coverage is 83.63636% with 18 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...rates/extensions/tedge_config_manager/src/actor.rs 75.00% 7 Missing and 6 partials ⚠️
...rates/extensions/tedge_config_manager/src/tests.rs 90.38% 0 Missing and 5 partials ⚠️
Additional details and impacted files

📢 Thoughts on this report? Let us know!

Copy link
Contributor

github-actions bot commented Aug 27, 2024

Robot Results

✅ Passed ❌ Failed ⏭️ Skipped Total Pass % ⏱️ Duration
499 0 2 499 100 1h28m13.132174999s

Copy link
Member

@rina23q rina23q 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. I have small cosmetic suggestion

Signed-off-by: Marcel Guzik <marcel.guzik@inetum.com>
Signed-off-by: Marcel Guzik <marcel.guzik@inetum.com>
Signed-off-by: Marcel Guzik <marcel.guzik@inetum.com>
Signed-off-by: Marcel Guzik <marcel.guzik@inetum.com>
@Bravo555 Bravo555 force-pushed the improve/config-manager-simplify-control-flow branch from 97d9d18 to a4f7a49 Compare August 29, 2024 14:41
@Bravo555 Bravo555 temporarily deployed to Test Pull Request August 29, 2024 14:41 — with GitHub Actions Inactive
Copy link
Contributor

@didier-wenzek didier-wenzek left a comment

Choose a reason for hiding this comment

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

Approved

@Bravo555 Bravo555 added this pull request to the merge queue Aug 29, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 29, 2024
@Bravo555 Bravo555 added this pull request to the merge queue Aug 29, 2024
Merged via the queue into thin-edge:main with commit 67b3f4f Aug 29, 2024
33 checks passed
@Bravo555 Bravo555 deleted the improve/config-manager-simplify-control-flow branch August 30, 2024 09:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactoring Developer value theme:configuration Theme: Configuration management
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants