Skip to content

🚸 Improve assertion error message if a group is not a valid subclass#425

Merged
tiangolo merged 4 commits intofastapi:masterfrom
chrisburr:better-assert-error
Aug 24, 2024
Merged

🚸 Improve assertion error message if a group is not a valid subclass#425
tiangolo merged 4 commits intofastapi:masterfrom
chrisburr:better-assert-error

Conversation

@chrisburr
Copy link
Copy Markdown
Contributor

With the latest typer release my CI (which runs a typer based script) started failing with:

Traceback (most recent call last):
  File "/builds/cburr/DIRAC/./integration_tests.py", line 750, in <module>
    app()
AssertionError

The cause turned out to be:

https://github.com/tiangolo/typer/blob/fac64ca03e9fda619710702c3364e233e0e644ac/typer/main.py#L505

which was triggered by my script contianing:

class NaturalOrderGroup(click.Group):
    """Group for showing subcommands in the correct order"""

    def list_commands(self, ctx):
        return self.commands.keys()

The fix is easy and just requires replacing click.Group with typer.core.TyperGroup however I think the assertion error should be made this clearer.

@github-actions
Copy link
Copy Markdown
Contributor

@svlandeg svlandeg added feature New feature, enhancement or request p3 labels Mar 6, 2024
Copy link
Copy Markdown
Member

@svlandeg svlandeg left a comment

Choose a reason for hiding this comment

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

It's a good idea to add this message to help developers with debugging. Thanks for your contribution!

@svlandeg svlandeg changed the title Make assertion error message clearer if group is not a valid subclass 🚸 Improve assertion error message if a group is not a valid subclass Apr 18, 2024
@tiangolo tiangolo merged commit 846dc41 into fastapi:master Aug 24, 2024
@tiangolo
Copy link
Copy Markdown
Member

Nice, thank you @chrisburr! 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature New feature, enhancement or request p3

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants