-
Notifications
You must be signed in to change notification settings - Fork 178
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
refactor(app): Allow app to request buildroot update token #3765
Conversation
Codecov Report
@@ Coverage Diff @@
## edge #3765 +/- ##
==========================================
+ Coverage 57.09% 57.24% +0.14%
==========================================
Files 817 820 +3
Lines 22545 22599 +54
==========================================
+ Hits 12873 12936 +63
+ Misses 9672 9663 -9
Continue to review full report at Codecov.
|
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.
Tested as in the PR notes, works for me as long as the update server is from 3.7.0 or previous, or 3.10.1 or subsequent (in the middle the shape of the capabilities
field in /server/update/health
is wrong)
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.
Tests look great! Awesome to have that epic coverage.
error?.payload.status === 409 | ||
) | ||
}), | ||
switchMap<RobotApiResponseAction, _, mixed>(action => { |
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.
Sometimes I'll name the action parameter after a filter something that describes it's uniqueness if the flow type doesn't narrow it down enough (e.g. errorAction
). Just a suggestion, though this is still pretty clear as is.
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.
Will do so in #3771
overview
This PR is a chunk of implementation work for un-sized tickets #3359 and #3361. It implements the following steps in the migration + update process:
changelog
review requests
The diff is a little inflated because I had to split apart
app/src/shell/buildroot.js
as it was getting too large. Sorry about that.A few decisions I made during implementation that will affect future work:
robotApi
module, so really I mean the epics and reducer are inapp/src/shell/buildroot
rather thanapp/src/robot-api/resources/update-server
or somethingThings I'd especially like feedback on:
update-epics.js
look?real-life testing instructions
Dispatch a
buildroot:START_UPDATE
action to a robot name, e.g.:Pro-tip: Use the Chrome console rather than the Redux devtools to dispatch the action so you have up-arrow history as you're testing. Copy-paste in the app's devtools is very broken at least on macOS for some reason
Then, keep an eye on the Redux devtools for the following actions depending on which path in the flow you're following:
buildroot:START_PREMIGRATION
,buildroot:PREMIGRATION_STARTED
,buildroot:PREMIGRATION_DONE
robotApi:START_UPDATE
robotApi:[REQUEST|RESPONSE|ERROR]__POST__/session/update[/migration]/begin
robotApi:[REQUEST|RESPONSE|ERROR]__POST__/session/update[/migration]/cancel