search

balena-io / balena-cli

The official balena CLI tool.
Apache License 2.0
453 stars 138 forks source link

`balena logs` doesn't work with ShortUUIDs, if they contain no alpha characters. #1456

Open hedss opened 5 years ago

hedss commented 5 years ago

Given short form UUID is available as an argument for every other command, logs should accept it as well:

$ balena logs 8051271
BalenaDeviceNotFound: Device not found: 8051271
...
$ balena logs 8051271485b202540d8fee37dd52a9df
[Logs]    [10/1/2019, 4:39:06 PM] Supervisor starting
[Logs]    [10/1/2019, 4:39:13 PM] Applying configuration change {"SUPERVISOR_POLL_INTERVAL":"900000"}
[Logs]    [10/1/2019, 4:39:13 PM] Applied configuration change {"SUPERVISOR_POLL_INTERVAL":"900000"}
[Logs]    [10/1/2019, 4:39:14 PM] Creating volume 'resin-data'
[Logs]    [10/1/2019, 4:39:14 PM] Creating network 'default'
pdcastro commented 4 years ago

Connects to:

srlowe commented 4 years ago

Turns out that this is only the case with ShortUUIDs that do not contain alpha characters (ShortUUIDs containing alpha characters work as expected).

This is caused by this issue in the SDK: https://github.com/balena-io/balena-sdk/issues/826

thgreasi commented 4 years ago

Reposting for the SDK issue:

This is probably a bug on the CLI side, where capitano is inferring & parsing numeric-only short uuids as numbers. That's already handled in other actions using either the normalizeUuidProp util function, or by directly using the propname_raw variant (I did fix a few of them back then :smile:) See: https://github.com/balena-io/balena-cli/search?q=normalizeUuidProp&unscoped_q=normalizeUuidProp

pdcastro commented 4 years ago

Just a note that if the issue is down to CLI/Capitano argument parsing, then another good solution is to convert the command from Capitano to oclif -- which is the plan in any case.

srlowe commented 4 years ago

@thgreasi Ah sorry, yeah you're right that the CLI is passing as number instead of string in those cases, so not an SDK issue - sorry for the false alarm :) (I was thrown by the ts string typing).

However it's already using normalizeUuidProp, and if I understand correctly, I don't see how this could help here. We have a single parameter uuidOrDevice, which can take both ShortUUIDs and Ids, both of which can be a string of digits with the same length, but one needs to be interpreted as a string, and the other a number. Isn't it impossible to distinguish between the two? (perhaps I am missing something...)

(@pdcastro I think this goes for switching to oclif too)

srlowe commented 4 years ago

After discussion with @thgreasi it seems the accepted solution in these ambiguous situations where the param could possibly be interpreted as either a shortUUID or an Id, is two calls to the SDK. One with the number typing (e.g. interpreting as Id), and then if no result a second call with the param converted to a string.

thgreasi commented 4 years ago

Just make sure to have this as a re-usable helper, like we did for releases, and to avoid repeating the code in several actions. See: https://github.com/balena-io/balena-cli/blob/ede72e1dd2244d469d5decdb68ab2981e6e0d976/lib/utils/normalization.ts#L30