Skip to content
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

Resource names that look like numbers are treated as IDs / short UUIDs #1842

Open
pdcastro opened this issue May 26, 2020 · 6 comments
Open

Comments

@pdcastro
Copy link
Contributor

pdcastro commented May 26, 2020

Some CLI commands currently distinguish between a resource name or ID or short UUID on the basis of whether it "looks like an integer". This used to be a side effect of the Capitano framework, and we thought that moving to oclif would solve it. Alas, after the migration to oclif, we still found ourselves applying a tryAsInteger() function (#1815#1817, #1841) because the affected commands do not currently have a way of distinguishing between a resource name (string) like 123456 and a resource ID (number) like 123456.

balena app 123456
balena tag set myTag --release 123456

The balena SDK, e.g. application.get(nameOrSlugOrId, [options]) has arguments like nameOrSlugOrId that distinguish between a name or ID by the argument data type (Javascript variable type): a number (ID) or string (name or slug). It is up to the CLI (or web dashboard) to determine whether the user input is a name or an ID. The CLI can then convert / cast the input to the required data type (123456 may be provided to the SDK as either a string or a number).

https://www.balena.io/docs/reference/sdk/node-sdk/#balena.models.application.get

I think the solution is to add command-line options to explicitly distinguish between ID / UUID / name, even if they are "optional options", in which absence the CLI applies tryAsInteger(). For example:

balena app 123456  # assumes number with `tryAsInteger()`
balena app --id 123456  # applies `parseInt()` (or similar)
balena app --name 123456  # do not convert to number
@pdcastro pdcastro changed the title Resource names that look like numbers are treated as IDs / UUIDs Resource names that look like numbers are treated as IDs / short UUIDs May 26, 2020
@srlowe
Copy link
Contributor

srlowe commented Jun 25, 2020

@pdcastro Another possible approach: We could have a convention where we use quotes to force the parser to treat them as strings in these ambiguous cases. e.g. balena app "12345"

@pdcastro
Copy link
Contributor Author

use quotes to force the parser to treat them as strings

One downside of this approach is that the quotes would have to be escaped on a command line (so they get through to the CLI process), and different shells have different conventions on how to escape quotes (e.g. bash vs cmd.exe vs PowerShell):

# bash:
$ balena app \"123456\"
$ balena app \'123456\'
$ balena app '"123456"'

$ cmd.exe uses the caret ^
$ balena app ^"123456^"

$ PowerShell uses the backtick `
$ balena app `"123456`"
$ balena app '"123456"'

On a "plain" command line, quoting quirks could be addressed with documentation / examples. In shell scripts (e.g. users' CI environments), getting quoting right can be trickier than one might think, when the whole command line is saved in a string variable requiring 2+ levels of escaping.

@pdcastro
Copy link
Contributor Author

Just raised another issue to illustrate what is clearly a bug:
#2077 BalenaApplicationNotFound: Application not found: 1234

@pdcastro
Copy link
Contributor Author

pdcastro commented Nov 2, 2020

Substantial further internal discussion was held on these internal/private threads:

A proof-of-concept PR was created for the "flags approach": #2080

Another PR was created to improve name / DB ID disambiguation: PR #2081 that resolves issue #2077.

Re application UUIDs, the balena SDK still needs to add support for it: balena-io/balena-sdk/issues/1016

A decision still needs to be made on how to unambiguously specify app names, UUIDs and numeric IDs on the command line.

@srlowe
Copy link
Contributor

srlowe commented Dec 15, 2020

Now we have app slugs, app ID support is planned to be removed shortly.

@pdcastro
Copy link
Contributor Author

Even with the possibility of using slugs, I think application UUIDs still have a role to play (as the proper replacement for numeric IDs). An important property of UUIDs is that they are immutable, while slugs change in unpredictable ways (with numeric suffixes) if the application's fancy name is changed. Consider, for example, users who have their own databases that keep track of applications and devices. An application UUID would be better suited than a slug as a foreign key.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants