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

Simplify the operation handling code #2880

Open
Bravo555 opened this issue May 15, 2024 · 1 comment
Open

Simplify the operation handling code #2880

Bravo555 opened this issue May 15, 2024 · 1 comment
Assignees
Labels
refactoring Developer value

Comments

@Bravo555
Copy link
Contributor

Is your refactoring request related to a problem? Please describe.

As originally described in the Further Comments section in the PR about using the File Transfer Service for log and config operations, the current architecture of the C8y mapper actor adds unnecessary complexity to the operation handling code, by:

  • spreading the execution of the operation between many different functions, making it harder to locally reason about the code
  • needing additional variables to keep the state necessary for handling of the operations, read by the different functions that run at different stages of the operation lifecycle

This:

  • introduces additional friction when making changes to operation handling, because one needs to update multiple functions that each handles a part of the operation's lifecycle, and the signature of the data being passed between these functions, frequently adding additional fields
  • mixes levels of abstraction: the actor needs to be aware of the different operations so that it can invoke correct functions to continue handling them

It concerns mainly the interaction between downloader and uploader actors, where every time we need to download/upload from within the operation, we need to send the request and return, in order not to block handling of other things.

Describe the solution you'd like

The preferred solution would be to handle the entirety of an operation in a module dedicated to that operation, without returning, saving state, and relying on other parts of the code to continue the execution. We should be able to run the future that handles the operation concurrently, as that's what async/await should allow.

This was already done for c8y_firmware_manager (#2600), now the aim would be to do something similar with c8y_mapper_ext and its operation handling.

Describe alternatives you've considered

Additional context

@Bravo555
Copy link
Contributor Author

Bravo555 commented Jul 11, 2024

#2909 eliminated the biggest problems with operation handling, but there are still some big improvements to do:

Follow-up tasks:

Additional checks:

@Bravo555 Bravo555 changed the title Simplify the operation handling code by replacing fragmented control flow with regular async/await Simplify the operation handling code Jul 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactoring Developer value
Projects
None yet
Development

No branches or pull requests

1 participant