Make specifying the version during configuration optional #1012
Conversation
dfunckt
force-pushed
the
optional-os-version
branch
4 times, most recently
from
94cd6c8 to
51452ac
Compare
|
@dfunckt Should we close this now that the respective sdk pr is closed? |
|
@thgreasi no, version will only be optional for the user, it will still be required in the SDK and API. This PR takes care of figuring the version automatically if the user didn't provide one. |
dfunckt
force-pushed
the
optional-os-version
branch
5 times, most recently
from
f694a1a to
42b9ee4
Compare
|
@thgreasi updated, can you please another look? |
lib/utils/helpers.ts
Outdated
| }) | ||
| .then(JSON.parse) | ||
| .catch(() => balena.models.device.getManifestBySlug(deviceType)); | ||
| const init: typeof _deviceInit = require('balena-device-init'); |
There was a problem hiding this comment.
Nit: If you make this function async, then this can become:
const init = await import('balena-device-init');and that way it'll automatically pick up the types by itself, so you can drop the _deviceInit import.
That would also let you use await below to flatten the promise chain there, which would make that a little clearer too.
The one constraint is that you can't import Bluebird as Promise any more in that case. Elsewhere we've just been renaming the import to Bluebird.
lib/utils/helpers.ts
Outdated
| image: string, | ||
| manifest: BalenaSdk.DeviceType, | ||
| ): Promise<string | null> { | ||
| const init: typeof _deviceInit = require('balena-device-init'); |
dfunckt
force-pushed
the
optional-os-version
branch
2 times, most recently
from
7675131 to
69cf2c9
Compare
dfunckt
force-pushed
the
optional-os-version
branch
from
69cf2c9 to
feaa8d6
Compare
|
@resin-ci retest |
1 similar comment
|
@resin-ci retest |
`version` used to be optional but it seems we recently had to make it a required parameter. However it really feels redundant when all it’s used for is to determine whether the command should issue a legacy user API key or a provisioning key. This makes version optional but tries to figure it out by itself by reading os-release from the image's boot partition. This is not foul-proof however, and while it'll work with most recent images it won't work with all and in that case it'll bail out and only then warn the user to specify it via the --version argument. Change-type: minor
dfunckt
force-pushed
the
optional-os-version
branch
from
feaa8d6 to
8291c96
Compare
|
@resin-ci retest |
versionused to be optional but it seems we recently had to make it a required parameter. However it really feels redundant when all it’s used for is to determine whether the command should issue a legacy user API key or a provisioning key.This makes
versionoptional but tries to figure it out by itself by readingos-releasefrom the image's boot partition. This is not foul-proof however, and while it'll work with most recent images (see balena-io-modules/balena-device-init#37) it won't work with all and in that case it'll bail out and only then warn the user to specify it via the--versionargument.Change-type: minor
Depends-on: balena-io-modules/balena-device-init#37