-
Notifications
You must be signed in to change notification settings - Fork 63
Workflow to restart microservices for a plugin #2667
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
base: main
Are you sure you want to change the base?
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2667 +/- ##
==========================================
- Coverage 79.25% 79.25% -0.01%
==========================================
Files 664 664
Lines 52882 52882
Branches 734 734
==========================================
- Hits 41912 41909 -3
- Misses 10890 10893 +3
Partials 80 80
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
8949229 to
78a506e
Compare
78a506e to
8bf0b1e
Compare
|
I like the implementation - Having a view microservices filter, and the control buttons is nice! Only thing wrong is too much trust in the state values. The State Values require cooperatively written microservices, and they aren't really a fixed set of keywords. State can be anything you want in a microservice, and can be undefined for microservices that don't provide microservice status. |
ryanmelt
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't base the success of the button presses on State. You can leave the colored state label, that is nice, but success/failure should just be based on the Enabled flag changing.
@ryanmelt it turns out that restarting with the microservice Here's some ideas/options...
Thoughts? |
|
Restarts just touch the object, and change the timestamp. You could have the restart as complete as soon as the timestamp changes. |
|
I agree with this approach--we don't actually need to detect a full restart cycle. Just showing the loading indicator for one state change gives the user feedback their button click worked, and it's still valid for a user to trigger a restart immediately again if they wanted to. |
|
I don't like the switches. If state is Running and enabled is false it's not running. That just means the last status received was running. Should probably not show the actual state if enabled is false, just show "stopped". |
|
Is that an api issue that should be addressed? Why would state keep emitting as "running" if enabled=false? |
|
No. Internally there are two models. MicroserviceModel and MicroserviceStatusModel. The first is the control model which should always be correct. The latter is the most recent status reported by the microservice. |
8bf0b1e to
a1bc1e8
Compare
|
Sounds good. Ready for review @ryanmelt. |
a1bc1e8 to
ed2f91f
Compare

Closes #2650
Demo
cosmos_microservices_demo.mp4
Edit: start button shows when enabled is false, otherwise show restart

Design choices / Changes
enabledvalueCould improve