Skip to content

Comments

v2 - Catch problems with [tool.poetry.scripts] entries#8898

Merged
radoering merged 9 commits intopython-poetry:masterfrom
sneakers-the-rat:script-error-msg
Feb 10, 2024
Merged

v2 - Catch problems with [tool.poetry.scripts] entries#8898
radoering merged 9 commits intopython-poetry:masterfrom
sneakers-the-rat:script-error-msg

Conversation

@sneakers-the-rat
Copy link
Contributor

@sneakers-the-rat sneakers-the-rat commented Jan 23, 2024

Pull Request Check List

Related-to: #issue-number-here (partially)

A continuation of: #7756 since the author seems to have left the chat

More informative error messages when scripts are misspecified.

Main difference is that the error message has been updated with more information, and the no-context information ("too many/too few values to unpacked") is removed.

The previously proposed error message was still a bit too cryptic, so I added separate cases for "too many colons" vs "colons absent" because they are different problems - with no colons, one might be trying to specify a whole python module because their script runs module-level code. I added a hint in that case to just wrap it in a main function with a suggestion of how to change pyproject.toml. The other case is just a warning that there are too many colons and shows what the current value is.

So for "no colons" with a pyproject.toml like:

[tool.poetry.scripts]
foo = "bar.bin.foo"
baz = "bar.bin.baz"

The error message is:

Bad script (baz): script needs to specify a function within a module like: module(.submodule):function
Instead got: bar.bin.baz
Hint: If the script depends on module-level code, try wrapping it in a main() function and modifying your script like:
baz = "bar.bin.baz:main"

and for a "too many colons" with a pyproject.toml like:

[tool.poetry.scripts]
foo = "foo::bar"
fux = "fuz.foo:bar:fux"

the error message is:

Bad script (foo): script needs to specify a function within a module like: module(.submodule):function
Instead got: foo::bar
Too many ":" found!

I also removed script specifiers like "foo.bar:baz.bin" because they were misleading about what the test handled.

It seems like the ultimate thing that needs to happen here is to add regex to the JSON schema file so that editors alert people before the time they try and install a package - https://github.com/python-poetry/poetry-core/blob/main/src/poetry/core/json/schemas/poetry-schema.json - but that's not for me today ;)

  • Added tests for changed code.
  • Updated documentation for changed code.

Secrus
Secrus previously requested changes Jan 27, 2024
Copy link
Member

@Secrus Secrus left a comment

Choose a reason for hiding this comment

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

One small issue, other than that, LGTM.

@radoering radoering merged commit 5f1d1a7 into python-poetry:master Feb 10, 2024
@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 13, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants