-
Notifications
You must be signed in to change notification settings - Fork 295
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
Modify Upgrade Responder client code to prepare for future changes #4062
Modify Upgrade Responder client code to prepare for future changes #4062
Conversation
4cad6be
to
4eafdb6
Compare
pkg/rancher-desktop/main/update/__tests__/LonghornProvider.spec.ts
Outdated
Show resolved
Hide resolved
pkg/rancher-desktop/main/update/__tests__/LonghornProvider.spec.ts
Outdated
Show resolved
Hide resolved
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.
Basically just for lint issues.
2af08cd
to
cc7a04a
Compare
cc7a04a
to
5de35fd
Compare
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.
Sorry, I haven't finished my review, but I'm running out of time right now.
I did find one or two places that look like bugs though, so submitting partial review now.
6adb259
to
a052bbc
Compare
58deb61
to
f4c6ff2
Compare
Marking as ready for review because it's getting too big. Also, it was mostly reviewed already and I don't want to confuse anyone by making more changes (more than I already have). I'm planning on adding |
38ce4c2
to
b028f54
Compare
b028f54
to
4e4afc1
Compare
4e4afc1
to
e7857ec
Compare
Sorry, I haven't finished the testing, and am offline for the next 4 days, so wanted to leave a few notes: I'm running influxdb in a separate VM, and the upgrade responder locally. I set the following environment variables: export RD_FORCE_UPDATES_ENABLED=1
export RD_MOCK_VERSION=1.6.0
export RD_UPGRADE_RESPONDER_URL=http://localhost:8314/v1/checkupgrade Using the lower mock version is so that the upgrade responder can suggest newer versions that actually exist as Github releases and can be downloaded. I added the following patch so that the version is passed to the upgrade responder: --- pkg/rancher-desktop/main/update/LonghornProvider.ts
+++ pkg/rancher-desktop/main/update/LonghornProvider.ts
@@ -350,7 +350,16 @@ export default class LonghornProvider extends Provider<LonghornUpdateInfo> {
}
}
- const queryResult = await queryUpgradeResponder(this.configuration.upgradeServer, this.updater.currentVersion);
+ let currentVersion = this.updater.currentVersion;
+
+ if (process.env.RD_MOCK_VERSION) {
+ const mockVersion = semver.coerce(process.env.RD_MOCK_VERSION);
+
+ if (mockVersion) {
+ currentVersion = mockVersion;
+ }
+ }
+ const queryResult = await queryUpgradeResponder(this.configuration.upgradeServer, currentVersion);
const { latest, unsupportedUpdateAvailable } = queryResult;
const requestIntervalInMinutes = queryResult.requestIntervalInMinutes || defaultUpdateIntervalInMinutes;
const requestIntervalInMs = requestIntervalInMinutes * 1000 * 60; I do get the following error on the console:
|
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.
Thanks, LGTM
Signed-off-by: Adam Pickering <[email protected]>
Signed-off-by: Adam Pickering <[email protected]>
Signed-off-by: Adam Pickering <[email protected]>
Signed-off-by: Adam Pickering <[email protected]>
Signed-off-by: Adam Pickering <[email protected]>
Signed-off-by: Adam Pickering <[email protected]>
Signed-off-by: Adam Pickering <[email protected]>
Signed-off-by: Adam Pickering <[email protected]>
dev-app-update.yml was not being honored in the previously-used version of electron-updater. The below PR is needed: electron-userland/electron-builder#7117 Signed-off-by: Adam Pickering <[email protected]>
Signed-off-by: Adam Pickering <[email protected]>
cab514d
to
bffa8c4
Compare
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.
Re-approving after rebase
Closes #4036.