Skip to content
This repository has been archived by the owner on Feb 28, 2021. It is now read-only.

cli: Check and notify user of a new version available #417

Open
NunoAlexandre opened this issue May 1, 2020 · 19 comments
Open

cli: Check and notify user of a new version available #417

NunoAlexandre opened this issue May 1, 2020 · 19 comments

Comments

@NunoAlexandre
Copy link
Contributor

NunoAlexandre commented May 1, 2020

Part of #435

Problem

A new version of the CLI might make the companion docs obsolete for the current version the users have running on their machine. For instance, with the introduction of #415, the docs will indicate running the new key-pair generate command while such command is not available in the current version that users have on their machine.

Proposal

Have the CLI lookup where the latest version available from an endpoint [0] and if outdated, print a message to the user to update i, providing the link to download the latest binary.

[0] I'd suggest us having a versions.json file in the registry.radicle.xyz portal, accessible through https://registry.radicle.xyz/versions.json where we have an entry per component, describing the latest version and download URL:

{
   "cli": {
       "version": <version>,
       "downloadURL": <download_url>,
    }
}

We could even have a update_type field being either optional or mandatory, to distinguish updates that simply refactor something and others that introduce breaking changes as in respect to the docs. Just an idea.

Breakdown

  • Define proper version in the radicle-registry-cli binary
  • Add versions.json file to the registry.radicle.xyz portal.
  • Have radicle-registry-cli parsing its bit from this remote versions.json monitor
  • Have radicle-registry-cli print a nice message recommending the user to update
@NunoAlexandre NunoAlexandre self-assigned this May 1, 2020
@NunoAlexandre NunoAlexandre changed the title cli: Check and notify user of new version available cli: Check and notify user of a new version available May 4, 2020
@geigerzaehler
Copy link

A couple of things to consider:

  • When in the CLI execution lifecycle to we want to check this?
  • How do we avoid degrading performance of the CLI? For example what if the requesting the version takes some time. Is that really something we want to do for key-pair list?
  • What is the process for updating versions.json? We should clearly document this.

Given all this considerations and the complexityI think we can address the problem more cheaply.

  1. We make sure that all reference documentation available from the web is also available through --help.
  2. We add a disclaimer to the documentation noting that it refers to the most recent version.

@NunoAlexandre
Copy link
Contributor Author

A couple of things to consider:

  • When in the CLI execution lifecycle to we want to check this?

Once everything is done, in main.

  • How do we avoid degrading performance of the CLI? For example what if the requesting the version takes some time. Is that really something we want to do for key-pair list?
  1. As said above, run it as the last thing.
  2. Set a very small timeout on the request

Requesting and parsing a super small json object should be extremely fast and simple, I can not imagine this being an issue in practice at all.

  • What is the process for updating versions.json? We should clearly document this.

Good point. First, we should have a better-defined version scheme. Now the releases are versioned after their date, but the binaries don't follow that properly. Once we have that, then the process would be to bump those versions in the updated binaries, release them, and finally bump the affected binaries in this versions.json file. This file should be the latest thing being updated, and it is not subjected to time-sensitivity, as is could even be updated one day later (should that happen, I mean) without affecting the user experience negatively.

Given all these considerations and the complexity think we can address the problem more cheaply.

I don't feel aligned with the opinion that this proposal is of such complexity. It's really a straightforward and cheap operation that adds value to the users and to the registry, promoting the usage of the latest versions of our software.

  1. We make sure that all reference documentation available from the web is also available through --help.

Can you expand? What do you mean by "reference documentation"? We can still have that as a criterion for our CLI docs, but it doesn't give us all the value that this proposal is after.

  1. We add a disclaimer to the documentation noting that it refers to the most recent version.

Again, that is something we should do anyway, but it doesn't have (and imho shouldn't) be a substitute for this proposal. They compliment each other.

One further consideration we haven't yet covered is what happens when the connection requesting the version fails or timeouts. In such a case, I am of the opinion that we should be silent and not bother the user. Alternatively, show a message: Couldn't check if there is a new version. Visit github.com/radicle-registry/releases ..."

@geigerzaehler
Copy link

I’m ok with the proposal. I just want to ensure that we make the right value/complexity trade offs. Before we tackle this I’d pick the low-hanging fruit and sort out the dependencies (i.e. release schemes).

Can you expand? What do you mean by "reference documentation"? We can still have that as a criterion for our CLI docs, but it doesn't give us all the value that this proposal is after.

You’re right. It does not encourage users to get the latest version. But it does mitigate the original problem mentioned in the description that the documentation on the web does not match the version of the binary.

The implementation suggestion makes sense. To keep the check unnoticeable we should probably go with a timeout of less than

Requesting and parsing a super small json object should be extremely fast and simple, I can not imagine this being an issue in practice at all.

The problem is latency. On my rather good network it takes an average of 0.3 seconds to curl the 750 bytes of https://registry.radicle.xyz. For a mobile 4g network it takes around 0.7 seconds compared to the command execution time of 0.01 seconds. For this reason I’d suggest to run the version check in parallel to the main execution but only print the result at the end.

Alternatively, show a message: Couldn't check if there is a new version. Visit github.com/radicle-registry/releases ..."

Not sure if that is a good idea. If you see it for the first time you might become distracted. If you’re on a slow network and see this frequently you’ll not see this

Requesting and parsing a super small json object should be extremely fast and simple, I can not imagine this being an issue in practice at all.

@CodeSandwich
Copy link
Contributor

CodeSandwich commented May 5, 2020

The problem

I think that this is a very risky design on multiple layers and it needs deep consideration.

First, there's the social layer. We're adding hidden communication with a centralized server. If it's impossible to disable, we can anger a lot of privacy focused users, who are our main target. If it's opt-out, it's going to be extremely frustrating for them. If it's opt-in, nobody's going to use it, it's easier to just visit our website.

Second, there's the technical layer. Thomas has already covered it well. For many small operations we will either slow them down to a crawl or make skipping the version check almost certain.

Third, there's the maintenance burden. It's yet another moving part to design, remember and consider during the updates. It will tie our binaries with yet another part of our infrastructure.

The mitigation

If we really want a notification mechanism built into the CLI, we can make it optional, but nagging. We can tell the users e.g. once a week, that he hasn't checked if there's a newer version. This can be printed on any CLI usage. The last check should be noted in a config file, so CLI knows when to nag again.

We can optionally add a special command check-update, which would download the said JSON file and tell if there's a newer version.

But why?

The more I think about this feature, the less convinced I am that it's going to bring a lot of value to the users. When the docs desynchronize with the CLI, the user will immediately know, that it's time to do the update.

@NunoAlexandre NunoAlexandre added this to the Registry Updates milestone May 8, 2020
This was referenced May 8, 2020
@CodeSandwich
Copy link
Contributor

I propose a simple solution:

The client should call onchain_runtime_version as the first thing after establishing a connection. The client knows, which versions it's compatible with. If the runtime version is not on that list, the user is asked to update. If it is, the client knows, which version of the RPC API it's talking to and which messages are supported (if the client is multi-versioned, e.g. it's prepared for an upcoming runtime update).

@NunoAlexandre
Copy link
Contributor Author

NunoAlexandre commented Jun 1, 2020

That solution is simple indeed, but it means that it won't work whenever the runtime is at any newer version unknown to the client, which will create unnecessary incompatibility.

@CodeSandwich
Copy link
Contributor

If a runtime version is known to the client, they're compatible, aren't they?

@NunoAlexandre
Copy link
Contributor Author

If a runtime version is known to the client, they're compatible, aren't they?

I meant "unkown", edited to reflect that.

@CodeSandwich
Copy link
Contributor

CodeSandwich commented Jun 1, 2020

it won't work whenever the runtime is at any newer version unknown to the client

That's the whole point. If there's a need to bump the spec_version, it's a breaking change and all the clients should be tested and adjusted. The runtime should not be the source of truth about the clients. There may be dozens of implementations and uses, so we can't guarantee that none of them will notice the change in behavior. If we could, we wouldn't update the spec_version in the first place.

@NunoAlexandre
Copy link
Contributor Author

it won't work whenever the runtime is at any newer version unknown to the client

That's the whole point. If there's a need to bump the spec_version, it's a breaking change

Not necessarily. We might bump spec_version because we introduced a new validation spec for some tx, which changes nothing regarding its compatibility with the runtime.

@geigerzaehler
Copy link

@CodeSandwich proposal is a good baseline solution. But I agree with @NunoAlexandre that it is too limiting in the long run.

@CodeSandwich
Copy link
Contributor

we introduced a new validation spec for some tx, which changes nothing regarding its compatibility with the runtime.

If it changes nothing regarding compatibility, we don't need a spec_version bump. If it makes at least one old tx invalid, we can't guarantee that no clients will be affected.

@NunoAlexandre
Copy link
Contributor Author

we introduced a new validation spec for some tx, which changes nothing regarding its compatibility with the runtime.

If it changes nothing regarding compatibility, we don't need a spec_version bump.

We do need to bump it or else we can't update the on-chain runtime.

If it makes at least one old tx invalid, we can't guarantee that no clients will be affected.

We can, we have lots of examples of that. For instance, #472 changes nothing to the client in terms of its compatibility with the affected runtime, while old tx unregistering a user would fail.

@CodeSandwich
Copy link
Contributor

CodeSandwich commented Jun 2, 2020

For instance, #472 changes nothing to the client in terms of its compatibility

It's not a good example. It changes the behavior and forces the client to change its logic and add some checks to avoid errors. Maybe our CLI was untouched, but we don't know what logic people are going to put into their clients.

I think that we were a bit too liberal and self-centered when deciding whether something is a breaking change.

@NunoAlexandre
Copy link
Contributor Author

For instance, #472 changes nothing to the client in terms of its compatibility

It's not a good example. It changes the behavior and forces the client to change its logic and add some checks to avoid errors. Maybe our CLI was untouched, but we don't know what logic people are going to put into their clients.

I think that we were a bit too liberal and self-centered when deciding whether something is a breaking change.

We are the only ones with a client of our runtime that we know, and our CLI is the only known client of our client that we can sensibly reason about. I don't see the point of using a hypothetical ad-hoc client that some users might have built at this stage.

Regarding the given example, you are right that it is a bad example, I was referring to no CLI breaking changes and that isn't correct here.

@CodeSandwich
Copy link
Contributor

We are the only ones with a client of our runtime that we know, and our CLI is the only known client of our client that we can sensibly reason about.

I think that it's a bad assumption, which will lead us to building tightly interwoven, locked-in software, MS-style. It will hurt us badly in a long run by limiting extensibility and interoperability.

@geigerzaehler What's your opinion on this topic?

@NunoAlexandre
Copy link
Contributor Author

We are the only ones with a client of our runtime that we know, and our CLI is the only known client of our client that we can sensibly reason about.

I think that it's a bad assumption, which will lead us to building tightly interwoven, locked-in software, MS-style. It will hurt us badly in a long run by limiting extensibility and interoperability.

To be clear, I like your thinking, but I prefer to first solve a problem I know that exists and that is well defined and palpable.

@CodeSandwich
Copy link
Contributor

Yes, we're discussing a solution to a specific problem and its relation to a bigger picture.

@geigerzaehler
Copy link

I have the impression the discussion is getting derailed. The problem described in this issue is very narrow and focuses on the docs and releases of the CLI without the relation to the runtime. I think we should focus on runtime compatibility and first explore what the various constraints and wants are there. @CodeSandwich and I can take care of this and write a summary. We should then discuss this with you @NunoAlexandre.

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

No branches or pull requests

3 participants