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

Controller/improvements and tests #166

Closed
wants to merge 4 commits into from

Conversation

NelsonVides
Copy link
Collaborator

To be merged after #162

Mainly this introduces separation between the controller and the users supervisor, and unit tests for them. See commits for details.

@NelsonVides NelsonVides force-pushed the controller/improvements_and_tests branch 2 times, most recently from e099115 to 24c223a Compare December 6, 2023 16:02
@NelsonVides NelsonVides force-pushed the controller/improvements_and_tests branch from 24c223a to f777f7b Compare December 11, 2023 23:35
@NelsonVides NelsonVides force-pushed the controller/improvements_and_tests branch from f777f7b to 2a07eb9 Compare December 12, 2023 08:44
@NelsonVides NelsonVides force-pushed the telemetry/timestamps_and_spans branch 5 times, most recently from b834956 to c494c65 Compare December 14, 2023 13:46
@NelsonVides NelsonVides force-pushed the controller/improvements_and_tests branch from 2a07eb9 to d308bb8 Compare December 14, 2023 13:48
Base automatically changed from telemetry/timestamps_and_spans to master December 14, 2023 14:03
@NelsonVides NelsonVides marked this pull request as ready for review December 14, 2023 14:04
@NelsonVides NelsonVides reopened this Dec 14, 2023
@NelsonVides NelsonVides force-pushed the controller/improvements_and_tests branch 2 times, most recently from f1e3d32 to a0d167b Compare December 14, 2023 14:38
@codecov-commenter
Copy link

Codecov Report

Attention: 12 lines in your changes are missing coverage. Please review.

Comparison is base (c494c65) 33.23% compared to head (a0d167b) 48.35%.
Report is 4 commits behind head on master.

Files Patch % Lines
src/amoc_users_sup.erl 79.16% 10 Missing ⚠️
src/amoc.erl 0.00% 1 Missing ⚠️
src/amoc_controller.erl 94.44% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           master     #166       +/-   ##
===========================================
+ Coverage   33.23%   48.35%   +15.12%     
===========================================
  Files          23       23               
  Lines         975     1001       +26     
===========================================
+ Hits          324      484      +160     
+ Misses        651      517      -134     

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

@NelsonVides NelsonVides marked this pull request as draft December 14, 2023 16:17
In the original implementation, the process is monitored by two supervisors, the
original amoc_users_sup, and the amoc_controller one. But this means that bursts
of ups and downs overflow the mailbox of _two_ processes, one of them being
actually the very critical controller.

With this reimplementation, only the amoc_users_sup tracks the processes, and
all requests from the controller are asynchronous. This ensures the controller
does not get blocked and can remain responsive to control requests.
@NelsonVides NelsonVides force-pushed the controller/improvements_and_tests branch from a0d167b to 53c1795 Compare December 15, 2023 23:19
@NelsonVides
Copy link
Collaborator Author

Split into #170, #171, #172

@NelsonVides NelsonVides deleted the controller/improvements_and_tests branch December 16, 2023 11:21
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