Deprecate instance name argument in favor of flag

[colinhacks]

Per the team call:

• Deprecate the passing of an instance name argument (edgedb instance stop myinst) in favor of a flag (edgedb instance stop -I myinst). Support for passing an instance name argument will be removed in a future release.
• Running naked instance commands (edgedb instance stop) should run against any project-linked instances if one exists.
• Passing any other connection flag other than -I should produce an error message explaining that only -I/--instance is supported for edgedb instance commands.

[tailhook]

Should we also warn on EDGEDB_PORT=1234 edgedb instance start ? Or even EDGEDB_INSTANCE=yyy edgedb instance start ?

[colinhacks]

If the command doesn't receive a -I flag and can't resolve a project-linked instance, a general purpose "Must provide an instance name or run this command inside a project directory" seems sufficient. Personally I think EDGEDB_INSTANCE=yyy edgedb instance start should work if possible.

[1st1]

Personally I think EDGEDB_INSTANCE=yyy edgedb instance start should work if possible.

That again blends -I command for instance commands with connection options... cc @elprans

[nsidnev]

Running naked instance commands (edgedb instance stop) should run against any project-linked instances if one exists.

Will this also deprecate edgedb project upgrade? If all edgedb instance commands take into account the existence of the project, then the edgedb instance upgrade command (as well as edgedb instance revert) should also change the version of EdgeDB in project config. Right?

If not, I find it a bit confusing and inconsistent (IMO).

[tailhook]

Passing any other connection flag other than -I should produce an error message explaining that only -I/--instance is supported for edgedb instance commands.

To make this work we have to define those options. They can easily be hidden from help, but hidden options are visible in completion. But I don't think completion for those options are good idea. I think fixing completion is quite hard task, and without fixing completion I'm not sure it's overall positive for the DX. So not sure if this worth the effort.

[tailhook]

Do we want -I for instance create and instance link ? I think they should have required argument for the name (so positional), no? (One possible alternative is to make instance create equivalent to project init when edgedb.toml is present, but I don't think this is a good idea)

Update: actually in link it's already optional, and asked for interactively if it's not present. Which is kinda good (-I is okay) and bad (instance link without a name have no deal with the current project like other commands).

[tailhook]

Similarly unlink and destroy might be confusing:

1. Currently destroy requires --force if used with a project-linked instance. Do we need --force for instance destroy without a name?
2. unlink is even more subtle: it's technically equivalent to project unlink --destroy, but only works with remote instances (we don't check linking to project there, but I'm sure this is a bug)

[colinhacks]

I think fixing completion is quite hard task, and without fixing completion I'm not sure it's overall positive for the DX.

sounds good, scratch that idea 👍

Do we want -I for instance create and instance link

I think both of these should accept an optional positional name argument, that falls back to an interactive prompt. I think it's fine that instance link with no name ignores the current project. link and create are special, in that they are "creating" instances that don't currently exist, and I think that's fairly easy to explain/conceptualize.

Do we need --force for instance destroy without a name?

I think we should

unlink is even more subtle

I think of instance unlinking and project unlinking as two very different operations. It's a bit unfortunate we used the same term for both. I think edgedb instance unlink should probably require a --force flag if it's unlinking a project-linked instance.

Will this also deprecate edgedb project upgrade

I'm in favor of this, but we'll likely keep the current behavior for the moment. There's a difference between automatically applying edgedb instance commands to a project-linked instance and actively changing project-based configuration files. Though admittedly this distinction is subtle.

[tailhook]

There is another subtle issue with link. We accept full connection params in link, so:

$ edgedb instance link -I test -H hello
error: The argument '--host <host>' cannot be used with '--instance <instance>'

And using -I as connection parameter might be useful to link a cloud instance to a local name, for example (under old semantics):

edgedb instance link my_instance -I my_cloud/my_instance

Or to alias instances:

edgedb instance link my_proj -I my_proj_v2

[colinhacks]

There is another subtle issue with link

I think both of these should accept an optional positional name argument

This is why create and link should use a positional argument for the instance name. I agree that this is how it should work:

edgedb instance link my_instance -I my_cloud/my_instance 

Any command that creates a new instance (or instance alias, in the case of link) should use a positional argument, and -I should be used when specifying an existing instance.