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

Make template upgrade kinder to old installs of common repos #1152

Merged

Conversation

itowlson
Copy link
Contributor

If a user has template installs from 0.8 or earlier, they will not have repo information. So if the user tries to upgrade them, they will instead get prompted to install-update them.

This PR attempts to provide a better experience for the Spin and Spin JS SDK repos, which are the ones users are most likely to have existing installs of. It tries, heuristically, to detect if the user has old installs of those repos, and if so inserts them into the list of repos to offer for upgrade. This means that a user who has installed from one of those repos in the past will now get the nice auto upgrade rather than being asked to use the incantation. E.g.:

# User installed from JS SDK repo with old Spin so no origin info
$ spin templates list --verbose
+------------------------------------------------------------------+
| Name      Description                             Installed from |
+==================================================================+
| http-js   HTTP request handler using Javascript                  |
| http-ts   HTTP request handler using Typescript                  |
+------------------------------------------------------------------+

# What sorcery is this? Upgrade still gives friendly selector instead of manual warning
 $ spin templates upgrade
> [ ] https://github.com/fermyon/spin-js-sdk (at spin/templates/v0.8)

There are a couple of ways this can go wrong:

  1. The heuristic is fooled if the user has uninstalled the templates we use for sniffing. In this case the user will be shown the remaining templates as "we couldn't tell where to upgrade from." This is not delightful but it is benign and a user who is engaged enough to curate individual templates can probably deal with it.

  2. If the user is sniffed as having the special repos, we hide all manual upgrade warnings. This is to avoid embedding a full list of special templates into the code. If the user has other templates that cannot be upgraded, from other repos or from local directories, no warning will be displayed for these. Once the user has upgraded their blessed repos, so that they have origin info, the warnings for the skipped templates will reappear. This behaviour is admittedly weird and hard to explain to users, but I think the number of people affected will be very small.

Otherwise it seems to work correctly though I'm a bit anxious about having logic this complicated. Hopefully we can remove it after a few releases!

Signed-off-by: itowlson <ivan.towlson@fermyon.com>
@itowlson itowlson force-pushed the templates-upgrade-ultimate-sorcery-xiii branch from dadd827 to 5c23571 Compare February 16, 2023 00:02
Signed-off-by: itowlson <ivan.towlson@fermyon.com>
Copy link
Member

@vdice vdice left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From a UX perspective, all appears well to me. (The selection instructions/prompt is very welcome!)

For testing, I didn't attempt to muck with any of the contents in the $XDG_HOME/spin/templates local store, more just removed them and tested scenarios of:

  • templates installed via --dir/relative paths (prompted to upgrade from fermyon/spin(-js-sdk) repo and latest templates tag)
  • templates installed from my fork of the repos (prompted to upgraded from same forked repo)
  • templates installed from fermyon/spin(-js-sdk) (prompted to upgrade from same repo and latest templates tag).

@itowlson itowlson merged commit c71c4d9 into fermyon:main Feb 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants