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

RFC 1001: Update vs upgrade #8

Open
elprans opened this issue May 4, 2020 · 9 comments
Open

RFC 1001: Update vs upgrade #8

elprans opened this issue May 4, 2020 · 9 comments

Comments

@elprans
Copy link
Member

elprans commented May 4, 2020

There are two concerns that RFC 1001 covers:

  1. The process of updating the installed server packages, which is done via edgedb server install --update in the current version of the RFC.

  2. The process of upgrading an EdgeDB server instance (i.e a data directory) to a new (major) EdgeDB version, which is done via edgedb server upgrade in the current version of the RFC.

@ambv pointed out that a lot of the options in edgedb server install do not actually apply to the --update mode (e. g. you cannot do --docker --update if the initial install was ran without --docker). Hence, the suggestion to have a separate command for installed package updates: edgedb server update. That, however, is very close to edgedb server upgrade and would be an endless source of confusion. Another idea from @vpetrovykh is to name the package update command as edgedb server patch.

Also, worth noting that edgedb server install --update is intended to update the installed package to the most recent minor version for a given major version.

Open questions:

  1. Should update be a standalone command or part of install?
  2. If standalone, what is the best name?
  3. What is the exact interaction of server upgrade with install and update?
@tailhook
Copy link
Contributor

tailhook commented May 5, 2020

I still think that keeping it in the install makes sense:

  1. It's what you teach in tutorial: edgedb server install. --help will help you update installation. I would say that we should have edgedb server install --latest in the tutorial to update and/or install latest major (I assume that install will bail out as soon as any single version is present).
  2. We should have server upgrade --install-latest for effortless upgrade, especially in docker (upgrade --patch can work, but since upgrade works with data, it's less clear what "patch" means)
  3. I don't see updating the installation as a very different operation, I think this is an installation of a new version (rather than "patching")

@vpetrovykh
Copy link
Member

I'd like to add a few words In defence of calling an update to the latest minor version a "patch". This procedure effectively gets you the latest patches to your current version, overwriting it. It is not possible to install several minor versions in parallel, so again, they are more like patches of a fundamentally same entity. They also don't require any fuss with data. Whereas major version upgrades are none of these things: major versions can coexist in parallel and they may require doing something to the data.

Rule of thumb is that you generally want to get patches ASAP, but may not necessarily want an upgrade in most software products and it seems to me that minor and major versions work like that for edgedb, too.

@tailhook
Copy link
Contributor

tailhook commented May 6, 2020

@vpetrovykh I agree with what you're saying. But I'm not sure that "patch" as a software update makes a lot of sense for developers who recently joined the field.

@ambv
Copy link
Contributor

ambv commented May 6, 2020

For the record, my suggestion for this was as follows:

e s install

  • enables to install a new specified version, you can say --latest if you don't know what that is
  • if a given version is already installed, idempotently declare success (return code 0)
  • if a given version cannot be installed side-by-side, return code 1 and direct people towards using e s upgrade
  • after successfully installing a version, we could always just add (unless --quiet) that "3 more versions installed" and/or "2 instances can be migrated to this version"

e s upgrade

  • by default upgrades all installed versions to their newest bugfix releases
  • if a higher non-bugfix release is available, informs: "a major version upgrade is possible, use --major to start it and migrate your instances"
  • it should be called "upgrade" because we might want to have "downgrade" in the future and "update" has no good equivalent

e s list [installed]

  • lists installed versions (and instances?)

e s list available

  • lists available versions

Why I think this is a good idea

Having one way to upgrade instances is safe. People won't run the wrong thing. And if they did mean --major and ran --minor by mistake, no foul, that should be entirely compatible. Plus, the tool will tell you that there's an available major upgrade path.

Having commands that only do one thing is easier to understand. It avoids combinations of arguments which don't make sense. It helps users state intent better. When they install a new vesion, the tool will tell them that this action did not upgrade their instances. They just have more versions available.

Other unsolved questions

e s upgrade --major would automatically migrate all instances from the old version to the new version. But if we ran e s install $NEW_VERSION before, should e s upgrade --major also migrate all instances?

I'm leaning towards yes but wanted to highlight this.

@elprans
Copy link
Member Author

elprans commented May 6, 2020

I actually like @ambv's approach, it makes sense to me. I would add an option to do selective per-instance --major upgrade, though.

@ambv
Copy link
Contributor

ambv commented May 6, 2020

Admit it, you just liked my masterful use of Markdown.

But yeah, we should do selective --major upgrades, that makes sense to me.

@elprans
Copy link
Member Author

elprans commented May 6, 2020

Admit it, you just liked my masterful use of Markdown.

Guilty as charged 😉

@1st1
Copy link
Member

1st1 commented May 7, 2020

Also +1 to @ambv's way.

Having one way to upgrade instances is safe

This is the key. Having two separate commands to update to minor and to major seems confusing to me.

@elprans
Copy link
Member Author

elprans commented May 7, 2020

OK, so it seems we have emerging consensus. I'll amend the RFC accordingly.

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

No branches or pull requests

5 participants