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

Command names with underscores must be invoked with dashes instead when upgrading from 6.7 to 7.0 #1123

Closed
CrescentFresh opened this issue Sep 26, 2018 · 18 comments
Milestone

Comments

@CrescentFresh
Copy link

CrescentFresh commented Sep 26, 2018

Click 7.0 is breaking upstream packages, in our case mschematool (issue reported there: aartur/mschematool#11).

After upgrading from 6.7 to 7.0, the init_db command can no longer be found.

$ mschematool xyz init_db
Usage: mschematool [OPTIONS] DBNICK COMMAND [ARGS]...
Try "mschematool --help" for help.

Error: No such command "init_db".
ERROR: Job failed: exit code 2
@CrescentFresh CrescentFresh changed the title Upgrade from click 6.7 to 7.0 causes "No such command" Upgrade from click 6.7 to 7.0 causes "Error: No such command" Sep 26, 2018
@ThiefMaster
Copy link
Member

You should not allow your CI to silently install a newer major version.

(FWIW, this could be a problem with click, but it's hard to say without seeing a minimal code example that works in click 6 and breaks in click 7)

@CrescentFresh
Copy link
Author

Hey man, we just install mschematool. Its dependencies are its dependencies.

Hopefully the author of that lib will chime in on the other issue.

@davidism
Copy link
Member

I can't reproduce this issue. Upgrading a sample project from 6.7 to 7.0 does not cause some commands not to be found. Please include a minimal reproducible example when reporting issues.

@adityanatraj
Copy link

we ran into this issue and noticed that commands that contained underscores are now expected with dashes. i'm trying to find out when this changed myself.

@ThiefMaster
Copy link
Member

As ugly as underscores in commands are, maybe it'd be better to normalize them to dashes and accept both?

@davidism davidism reopened this Sep 26, 2018
@davidism
Copy link
Member

davidism commented Sep 26, 2018

This was basically the one commit (5d1df0e) from @mitsuhiko that I couldn't find a related issue or PR for.

It's a breaking change, but this is a major release, so that's acceptable. If I had realized that is was a breaking change I would have made it more prominent in the changelog.

I definitely don't think it's a good idea to automatically provide multiple spellings for commands, but maybe we need a switch for the behavior while we take some time to deprecate it?

@davidism davidism changed the title Upgrade from click 6.7 to 7.0 causes "Error: No such command" Replacing underscores with dashses causes command names to change from 6.7 to 7.0 Sep 26, 2018
@davidism davidism added this to the 7.1 milestone Sep 26, 2018
@di
Copy link

di commented Sep 26, 2018

I'm experiencing this as a regression as well, here's a test case that passes with click==6.7 and fails with click==7.0:

import click
import click.testing

runner = click.testing.CliRunner()

thing = 'foobar'


@click.group()
@click.pass_context
def foo(ctx):
    ctx.obj = thing


@foo.command()
@click.pass_obj
def cli_test_command(obj):
    assert obj is thing


result = runner.invoke(foo, ["cli_test_command"])

assert result.exit_code == 0

Is this expected (i.e., is in the changelog) or a bug?

EDIT: Just saw that this is expected, but was missed in the changelog.

di added a commit to pypi/warehouse that referenced this issue Sep 26, 2018
@CrescentFresh
Copy link
Author

CrescentFresh commented Sep 26, 2018

So if I understand this correctly:

a) commit 5d1df0e intentionally broke bc with commands with underscores in them, instead requiring dashes
2) python methods to my knowledge cannot have dashes in them, yet these methods are still the thing command authors are expected to decorate with click

The combination of the above = boom.

So are command authors still recommended to use underscores in their method names or not?

@ThiefMaster
Copy link
Member

That's the whole point of the change: Have dashes in the commands while having valid python code (underscores)

@CrescentFresh
Copy link
Author

CrescentFresh commented Sep 26, 2018

How is a user of a tool that uses click under the hood supposed to know this? As you point out command authors don't have to do anything differently, yet the users of said tooling - who don't even know that click is being used under the hood - have to know to not use underscores in command line parameters. They're not going to read the changelog of a package they don't know exists.

@mitsuhiko
Copy link
Contributor

So the core of the issue here is that people don't pin dependencies. Click 7 is not compatible with Click 6 the same way it was not compatible with Click 5. I think this time around it's just more noticable.

@ThiefMaster
Copy link
Member

Like @davidism mentioned, this was poorly communicated in the changelog. Regardless of this, tool developers should not allow any future major version change when specifying their requirements, to avoid such surprises.

@mitsuhiko
Copy link
Contributor

One workaround I see is that we could also look for commands with dashes when someone tries a command with underscores.

@ThiefMaster
Copy link
Member

What about doing that in both directions? (and throw an exception if someone registers two different versions that only differ in dash/underscore usage)

@adityanatraj
Copy link

adityanatraj commented Sep 26, 2018

@davidism thanks for the commit ref and the explanation. I think it could have been more prominently described in the changelog (given python naming conventions would imply an insignificant number of people would hit this) but I do understand it was a major release. On our side, probably we should have pinned our dependencies better -_-.

perhaps it makes sense to attempt both for some interim period? of course, you'd have to dedupe (and raise some exception) when someone goes wild and attempts to register both styles...

@di nice to see you!

di pushed a commit to pypi/warehouse that referenced this issue Sep 26, 2018
* Update click from 6.7 to 7.0

* Update click from 6.7 to 7.0

* Update test for breaking change in click

pallets/click#1123
@CrescentFresh CrescentFresh changed the title Replacing underscores with dashses causes command names to change from 6.7 to 7.0 Users required to use dashes instead of underscores in command names when upgrading from 6.7 to 7.0 Sep 26, 2018
@PorkShoulderHolder
Copy link

PorkShoulderHolder commented Sep 27, 2018

It would have made sense to accept both formats and issue a DeprecationWarning when underscored commands were called, and then phase out underscores in a later version, issuing an error that describes the reason precisely (i.e. "now you gotta put dashes in version 7.0") IMHO. This change broke a bunch of commands for me...

@davidism
Copy link
Member

davidism commented Sep 27, 2018

You can still use underscores by specifying the command names explicitly. It's just that the default is now to replace with dashses, to look more consistent with other CLI tools, whereas before you would have had to specify those explicitly.

So if you want to upgrade your app to 7.0 but continue using the old names, you can name them explicitly, or use a Command subclass that doesn't have the new behavior. There are upgrade strategies here, it just means Click 7.0 is not a drop-in upgrade in some cases.

@davidism davidism changed the title Users required to use dashes instead of underscores in command names when upgrading from 6.7 to 7.0 Replacing underscores with dashses causes command names to change from 6.7 to 7.0 Sep 27, 2018
@CrescentFresh CrescentFresh changed the title Replacing underscores with dashses causes command names to change from 6.7 to 7.0 Command names with underscores must be invoked with dashes instead when upgrading from 6.7 to 7.0 Sep 27, 2018
@pallets pallets deleted a comment from CrescentFresh Sep 27, 2018
@pallets pallets locked and limited conversation to collaborators Sep 27, 2018
@davidism
Copy link
Member

davidism commented Feb 22, 2020

At this point, it's been so long since 7.0 that I think the issue is well understood and handled in projects. Ideally, we would have addressed this sooner, but things got away from me and no one followed up with it. I'm sorry for that.

Projects have had a few options. First, as pointed out earlier, you should always pin your dependencies in order to control how your project is installed and when you upgrade. You can specify command names manually, like @click.command("name_with_underscores"), or update documentation to use the names with dashes. Or there's a nice third option, that unfortunately I didn't figure out until recently. I wish I had been able to post it sooner.

During recent discussion in chat, we were considering adding a match_underscore_names context setting that would replaces dashes with underscores if the name wasn't found. When I went to implement it, I noticed that this behavior already exists in a more general fashion. If a command is not found and the context has a token_normalize_func set, that will be called to potentially get a new name to look up.

import click

def normalize(name):
    return name.replace("_", "-")

@click.group(context_settings={"token_normalize_func": normalize})
def group():
    pass

@group.command()
def show_greeting():
    click.echo("Hello, World!")

if __name__ == "__main__":
    group()
$ python example.py show_greeting
Hello, World!

I will ensure this is prominent in the docs, possibly as part of #1221. At this point, I don't think it's necessary to add any code to Click, so I'm going to close this.

azogue pushed a commit to azogue/python-miio that referenced this issue Mar 19, 2020
The command line interface is different for versions <7 (underscores instead of dash, see pallets/click#1123). The documentation only documents the commands with dash. So click>=7 is required.
lguohan added a commit to lguohan/sonic-utilities that referenced this issue Mar 21, 2020
Starting click 7.0. The default behavior of a command with under
scores will be replace with dashes.

this is to address the above default behavior change, so that
the command remains the same.

more details can be found:

pallets/click#1123
lguohan added a commit to sonic-net/sonic-utilities that referenced this issue Mar 22, 2020
Starting click 7.0. The default behavior of a command with under
scores will be replace with dashes.

this is to address the above default behavior change, so that
the command remains the same.

more details can be found:

pallets/click#1123
abdosi pushed a commit to sonic-net/sonic-utilities that referenced this issue Mar 24, 2020
Starting click 7.0. The default behavior of a command with under
scores will be replace with dashes.

this is to address the above default behavior change, so that
the command remains the same.

more details can be found:

pallets/click#1123
delgadom added a commit to ClimateImpactLab/jrnr that referenced this issue Apr 21, 2020
abdosi pushed a commit to abdosi/sonic-utilities that referenced this issue Aug 4, 2020
Starting click 7.0. The default behavior of a command with under
scores will be replace with dashes.

this is to address the above default behavior change, so that
the command remains the same.

more details can be found:

pallets/click#1123
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 13, 2020
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

7 participants