SDA-4182: Add GPOs to auto update channel #1896

NguyenTranHoangSym · 2023-07-18T05:52:42Z

Description

Auto-update params defined via GPOs
Read channel from registry and apply to SDA

Related PRs

KiranNiranjan · 2023-07-18T05:59:29Z

src/app/auto-update-handler.ts

-    } else if (isWindowsOS) {
-      this.autoUpdater = new NsisUpdater(opts);
-    }
+    this.getGenericServerOptions().then((opts) => {


This is only for Windows right?

yes windows-only

Then I think you need to wrap RegistryStore.getRegistry and retrieveWindowsRegistry inside isWindowsOS

KiranNiranjan · 2023-07-18T06:02:30Z

src/app/auto-update-handler.ts

-      ? 'beta'
-      : autoUpdateChannel;
+
+    const registryAutoUpdate = RegistryStore.getRegistry();


This is also for Windows only

KiranNiranjan · 2023-07-18T06:03:31Z

src/app/auto-update-handler.ts

@@ -221,16 +237,24 @@ export class AutoUpdate {
    return updateUrl;
  };

-  private getGenericServerOptions = (): GenericServerOptions => {
+  private getGenericServerOptions = async (): Promise<GenericServerOptions> => {
+    await retrieveWindowsRegistry();


This is also for Windows only

KiranNiranjan · 2023-07-18T06:56:32Z

src/app/auto-update-handler.ts

+        const registryAutoUpdate = RegistryStore.getRegistry();
+        const identifiedChannel = [
+          EChannelRegistry.BETA,
+          EChannelRegistry.LATEST,
+        ].includes(registryAutoUpdate.currentChannel)
+          ? registryAutoUpdate.currentChannel
+          : '';
+        const finalAutoUpdateChannel =
+          isWindowsOS && identifiedChannel
+            ? identifiedChannel
+            : autoUpdateChannel;


Suggested change

const registryAutoUpdate = RegistryStore.getRegistry();

const identifiedChannel = [

EChannelRegistry.BETA,

EChannelRegistry.LATEST,

].includes(registryAutoUpdate.currentChannel)

? registryAutoUpdate.currentChannel

: '';

const finalAutoUpdateChannel =

isWindowsOS && identifiedChannel

? identifiedChannel

: autoUpdateChannel;

let finalAutoUpdateChannel = autoUpdateChannel;

if (isWindowsOS) {

const registryAutoUpdate = RegistryStore.getRegistry();

const identifiedChannel = [

EChannelRegistry.BETA,

EChannelRegistry.LATEST,

].includes(registryAutoUpdate.currentChannel)

? registryAutoUpdate.currentChannel

: '';

if (identifiedChannel) {

finalAutoUpdateChannel = identifiedChannel;

}

}

KiranNiranjan

on minor change and it LGTM

sbenmoussati · 2023-07-18T07:17:47Z

src/app/auto-update-handler.ts

+        ? registryAutoUpdate.currentChannel
+        : '';
+      const channel =
+        isWindowsOS && identifiedChannel


isWindowsOS not needed here

sbenmoussati · 2023-07-18T07:20:48Z

src/app/auto-update-handler.ts

+      await retrieveWindowsRegistry();
+
+      const registryAutoUpdate = RegistryStore.getRegistry();
+      const identifiedChannel = [


Suggested change

const identifiedChannel = [

const identifiedChannelFromRegistry = [

sbenmoussati · 2023-07-18T07:21:30Z

src/app/auto-update-handler.ts

+        isWindowsOS && identifiedChannel
+          ? identifiedChannel
+          : autoUpdateChannel;
+      userAutoUpdateChannel = betaAutoUpdateChannelEnabled ? 'beta' : channel;


GPO has a higher prio than ACP (betaAutoUpdateChannelEnabled), so normally it should be like:

userAutoUpdateChannel = identifiedChannelFromRegistry ? identifiedChannelFromRegistry : betaAutoUpdateChannelEnabled ? 'beta' : channel;

src/app/auto-update-handler.ts

src/app/stores/registry-store.ts

sbenmoussati · 2023-07-18T08:13:11Z

src/app/auto-update-handler.ts

-        }
-        if (isMac) {
-          config.backupGlobalConfig();
+        const { autoUpdateChannel } = config.getConfigFields([


Duplicated logic here, I would rather put it in a dedicated function that would return the channel.
IMO this function should be called directly in
this.autoUpdater.on('update-available', (info) => { ... })
this.autoUpdater.on('download-progress', (info) => { ... })
this.autoUpdater.on('update-downloaded', (info) => { ... })

NguyenTranHoangSym requested review from KiranNiranjan and sbenmoussati July 18, 2023 05:52

NguyenTranHoangSym self-assigned this Jul 18, 2023

KiranNiranjan requested changes Jul 18, 2023

View reviewed changes

KiranNiranjan reviewed Jul 18, 2023

View reviewed changes

NguyenTranHoangSym force-pushed the SDA-4182 branch 2 times, most recently from 38760ce to cd8fd61 Compare July 18, 2023 06:36

NguyenTranHoangSym requested a review from KiranNiranjan July 18, 2023 06:47

KiranNiranjan reviewed Jul 18, 2023

View reviewed changes

KiranNiranjan approved these changes Jul 18, 2023

View reviewed changes

NguyenTranHoangSym force-pushed the SDA-4182 branch from cd8fd61 to 5431fae Compare July 18, 2023 07:07

sbenmoussati reviewed Jul 18, 2023

View reviewed changes

src/app/auto-update-handler.ts Outdated Show resolved Hide resolved

NguyenTranHoangSym force-pushed the SDA-4182 branch from 5431fae to 5620b71 Compare July 18, 2023 07:22

sbenmoussati reviewed Jul 18, 2023

View reviewed changes

src/app/stores/registry-store.ts Outdated Show resolved Hide resolved

NguyenTranHoangSym force-pushed the SDA-4182 branch from 5620b71 to b484536 Compare July 18, 2023 07:33

NguyenTranHoangSym requested a review from sbenmoussati July 18, 2023 07:35

SDA-4182: Add GPOs to auto update channel

7c8fd05

NguyenTranHoangSym force-pushed the SDA-4182 branch from b484536 to 7c8fd05 Compare July 18, 2023 07:45

sbenmoussati reviewed Jul 18, 2023

View reviewed changes

sbenmoussati merged commit 15056f1 into finos:main Jul 18, 2023
5 of 6 checks passed

NguyenTranHoangSym added a commit to NguyenTranHoangSym/SymphonyElectron that referenced this pull request Sep 5, 2023

SDA-4182: Add GPOs to auto update channel (finos#1896)

9d821c4

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SDA-4182: Add GPOs to auto update channel #1896

SDA-4182: Add GPOs to auto update channel #1896

NguyenTranHoangSym commented Jul 18, 2023

KiranNiranjan Jul 18, 2023

NguyenTranHoangSym Jul 18, 2023

KiranNiranjan Jul 18, 2023

KiranNiranjan Jul 18, 2023

KiranNiranjan Jul 18, 2023

KiranNiranjan Jul 18, 2023

KiranNiranjan left a comment

sbenmoussati Jul 18, 2023

sbenmoussati Jul 18, 2023

sbenmoussati Jul 18, 2023 •

edited

Loading

sbenmoussati Jul 18, 2023 •

edited

Loading

	const identifiedChannel = [
	const identifiedChannelFromRegistry = [

SDA-4182: Add GPOs to auto update channel #1896

SDA-4182: Add GPOs to auto update channel #1896

Conversation

NguyenTranHoangSym commented Jul 18, 2023

Description

Related PRs

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KiranNiranjan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sbenmoussati Jul 18, 2023 • edited Loading

Choose a reason for hiding this comment

sbenmoussati Jul 18, 2023 • edited Loading

Choose a reason for hiding this comment

sbenmoussati Jul 18, 2023 •

edited

Loading

sbenmoussati Jul 18, 2023 •

edited

Loading