Enable OS Updates to pre-release versions of higher base semver #1398

thgreasi · 2024-01-04T14:47:48Z

Depends-on: balena-io-modules/balena-hup-action-utils#34
Depends-on: https://github.com/balena-io/balena-proxy/pull/1098
Update balena-hup-action-utils from 5.0.0 to 6.1.0

Change-type: minor

Resolves: #
HQ:
See:
Depends-on:
Change-type: major|minor|patch

Contributor checklist

Includes tests
Includes typings
Includes updated documentation
Includes updated build output

otaviojacobi · 2024-01-19T12:17:16Z

src/models/os.ts

 							$filter: {
-								is_final: true,
+								...(filterOptions !== 'include_draft' && { is_final: true }),


maybe filterOptions === 'default' is more clear?

Page- · 2024-01-19T12:18:55Z

src/models/os.ts

@@ -392,6 +397,7 @@ const getOsModel = function (
 	 * @memberof balena.models.os
 	 *
 	 * @param {String|String[]} deviceTypes - device type slug or array of slugs
+	 * @param {Boolean} [includeDraft=false] - Whether pre-releases should be included in the results


I think this could make more sense as an options object so that it can also cover the potential cases of production vs draft vs old/legacy vs known issues, etc without having to add a bunch of individual arguments

thgreasi · 2024-01-19T15:17:02Z

src/models/os.ts

-		async (deviceTypes: string[], listedByDefault: boolean | null) => {
+		async (
+			deviceTypes: string[],
+			filterOptions: 'default' | 'include_draft' | 'all',


authDependentMemoizer is atm using primitive: true and doesn't expose a normalizer, so I've left it for the next time we need to add an extra property for getAvailableOsVersions.

…se semver Change-type: minor Depends-on: balena-io/balena-sdk#1398 See: https://balena.fibery.io/Work/Task/cli-Enable-OS-Updates-to-pre-release-OS-versions-1751

myarmolinsky · 2024-01-23T12:08:43Z

tests/integration/models/device.spec.ts

+										os_version: osVersion,
+										os_variant: osVariant,
+									},
+									'2.29.2-1704382618288+rev1.prod',


Can we please have comments on why these specific OS versions were chosen? Same on line 3999 and 3960 (and anywhere else where we put a specific OS version)

B/c that's the only one atm on prod that is draft, and since it's old enough I don't expect it to become finalized ever :)

Ends up the versions in this file are random, so I added a comment about that right at the start of the describe().

myarmolinsky · 2024-01-23T12:09:40Z

tests/integration/models/device.spec.ts

+						[['balenaOS 2.28.0+rev1', 'prod']].forEach(function ([
+							osVersion,
+							osVariant,
+						]) {
+							expect(() =>


Why are we using an array and forEach when it's just one item?

myarmolinsky · 2024-01-23T12:11:54Z

tests/integration/models/device.spec.ts

+						[['balenaOS 2.28.0-1704382553234', 'prod']].forEach(function ([
+							osVersion,
+							osVariant,
+						]) {
+							expect(() =>


Why are we using an array and forEach when it's just one item?

myarmolinsky · 2024-01-23T12:12:02Z

tests/integration/models/device.spec.ts

+						[['balenaOS 2.28.0-1704382553234', 'prod']].forEach(function ([
+							osVersion,
+							osVariant,
+						]) {
+							expect(() =>


Why are we using an array and forEach when it's just one item?

myarmolinsky · 2024-01-23T12:23:16Z

src/models/os.ts

+			filterOptions: 'default' | 'include_draft' | 'all',
+		) => {
 			return await _getAllOsVersions(
 				deviceTypes,
-				listedByDefault
-					? {
+				filterOptions === 'all'
+					? undefined
+					: {
 							$filter: {
-								is_final: true,
+								...(filterOptions === 'default' && { is_final: true }),


I'd prefer either a comment pointing out that default is finalized only or to replace default with finalized to make it clearer. (tbh I prefer replacing default with finalized most for clarity)

myarmolinsky · 2024-01-23T12:27:18Z

src/models/os.ts

@@ -404,13 +411,14 @@ const getOsModel = function (
 	 */
 	async function getAvailableOsVersions(
 		deviceTypes: string[] | string,
+		options?: { includeDraft?: boolean },


Maybe this and line 900 should be the options that _memoizedGetAllOsVersions expects for its second parameter

See: #1398 (comment)

Moreover _memoizedGetAllOsVersions is used by both getAvailableOsVersions & getAllOsVersions.
Till now I couldn't find a nice way to combine them :(

myarmolinsky · 2024-01-23T12:34:21Z

typings/pinejs-client-core.d.ts

Nothing changed? Just reformatted?

myarmolinsky · 2024-01-23T12:46:12Z

tests/integration/models/os.spec.ts

+				expect(draftVersions).to.have.lengthOf(0);
+			});
+
+			it('should include draft OS versions when the respective flag is not used [string device type argument]', async () => {


Suggested change

it('should include draft OS versions when the respective flag is not used [string device type argument]', async () => {

it('should include draft OS versions when the respective flag is used [string device type argument]', async () => {

…se semver Change-type: minor Depends-on: balena-io/balena-sdk#1398 See: https://balena.fibery.io/Work/Task/cli-Enable-OS-Updates-to-pre-release-OS-versions-1751

myarmolinsky · 2024-01-23T13:34:34Z

LGTM

Change-type: minor

Update balena-hup-action-utils from 5.0.0 to 6.1.0 Change-type: minor

…ases Change-type: minor

Change-type: patch

…se semver Change-type: minor Depends-on: balena-io/balena-sdk#1398 See: https://balena.fibery.io/Work/Task/cli-Enable-OS-Updates-to-pre-release-OS-versions-1751

thgreasi changed the title ~~Update balena-hup-action-utils to 6.1.0~~ Enable OS Updates to pre-release versions of higher base semver Jan 4, 2024

thgreasi force-pushed the draft-hup branch 2 times, most recently from 85a9481 to ded8ea6 Compare January 4, 2024 15:57

thgreasi force-pushed the draft-hup branch 6 times, most recently from 536b6dc to b77818b Compare January 19, 2024 11:39

thgreasi requested a review from otaviojacobi January 19, 2024 11:41

thgreasi marked this pull request as ready for review January 19, 2024 11:41

thgreasi requested review from JSReds and myarmolinsky January 19, 2024 11:41

otaviojacobi reviewed Jan 19, 2024

View reviewed changes

Page- reviewed Jan 19, 2024

View reviewed changes

thgreasi requested review from otaviojacobi and Page- January 19, 2024 14:42

thgreasi force-pushed the draft-hup branch 3 times, most recently from 5af99af to 83f65f3 Compare January 19, 2024 15:13

thgreasi commented Jan 19, 2024

View reviewed changes

flowzone-app bot enabled auto-merge January 19, 2024 15:38

thgreasi mentioned this pull request Jan 22, 2024

device os-update: Enable updates to pre-release versions of higher base semver balena-io/balena-cli#2722

Merged

myarmolinsky reviewed Jan 23, 2024

View reviewed changes

thgreasi force-pushed the draft-hup branch from 83f65f3 to 728448f Compare January 23, 2024 13:12

myarmolinsky marked this pull request as draft January 23, 2024 13:34

auto-merge was automatically disabled January 23, 2024 13:34
Pull request was converted to draft

thgreasi added 4 commits January 23, 2024 15:48

os.getAvailableOsVersions: Add the option to include draft releases

b8e795a

Change-type: minor

Enable OS Updates to pre-release versions of higher base semver

38360a8

Update balena-hup-action-utils from 5.0.0 to 6.1.0 Change-type: minor

os.getSupportedOsUpdateVersions: Add the option to include draft rele…

87b3187

…ases Change-type: minor

Update the deviceType.getInstructions tests

3fd5f7d

Change-type: patch

thgreasi force-pushed the draft-hup branch from 728448f to 3fd5f7d Compare January 23, 2024 13:49

thgreasi marked this pull request as ready for review January 23, 2024 14:10

thgreasi enabled auto-merge January 23, 2024 14:15

thgreasi merged commit 5318a7b into master Jan 23, 2024
50 checks passed

thgreasi deleted the draft-hup branch January 23, 2024 14:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable OS Updates to pre-release versions of higher base semver #1398

Enable OS Updates to pre-release versions of higher base semver #1398

thgreasi commented Jan 4, 2024 •

edited

Loading

otaviojacobi Jan 19, 2024 •

edited

Loading

Page- Jan 19, 2024

thgreasi Jan 19, 2024

myarmolinsky Jan 23, 2024 •

edited

Loading

thgreasi Jan 23, 2024 •

edited

Loading

thgreasi Jan 23, 2024

myarmolinsky Jan 23, 2024

myarmolinsky Jan 23, 2024

myarmolinsky Jan 23, 2024

myarmolinsky Jan 23, 2024

myarmolinsky Jan 23, 2024

thgreasi Jan 23, 2024

thgreasi Jan 23, 2024

myarmolinsky Jan 23, 2024

thgreasi Jan 23, 2024

myarmolinsky Jan 23, 2024

myarmolinsky commented Jan 23, 2024

	it('should include draft OS versions when the respective flag is not used [string device type argument]', async () => {
	it('should include draft OS versions when the respective flag is used [string device type argument]', async () => {

Enable OS Updates to pre-release versions of higher base semver #1398

Enable OS Updates to pre-release versions of higher base semver #1398

Conversation

thgreasi commented Jan 4, 2024 • edited Loading

Contributor checklist

otaviojacobi Jan 19, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

myarmolinsky Jan 23, 2024 • edited Loading

Choose a reason for hiding this comment

thgreasi Jan 23, 2024 • edited Loading

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

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

myarmolinsky commented Jan 23, 2024

thgreasi commented Jan 4, 2024 •

edited

Loading

otaviojacobi Jan 19, 2024 •

edited

Loading

myarmolinsky Jan 23, 2024 •

edited

Loading

thgreasi Jan 23, 2024 •

edited

Loading