Skip to content

Add option to fail on unknown keys.#506

Closed
Carreau wants to merge 1 commit into
pypa:mainfrom
Carreau:505
Closed

Add option to fail on unknown keys.#506
Carreau wants to merge 1 commit into
pypa:mainfrom
Carreau:505

Conversation

@Carreau

@Carreau Carreau commented Jan 7, 2022

Copy link
Copy Markdown
Contributor

As a draft, config, and naming to discuss.

I'm tempted to make it strict by default, and have the option to ignore
if ever there are extra keys added.

Closes #505

As a draft, config, and naming to discuss.

I'm tempted to make it strict by default, and have the option to ignore
if ever there are extra keys added.

Closes pypa#505
@takluyver

Copy link
Copy Markdown
Member

I'd certainly be OK with having the strict option by default, and I think I'd go one step further and not make an option at all.

I just remembered we already have an environment variable FLIT_ALLOW_INVALID as an escape hatch for metadata that fails validation. That applies to a separate validation step which occurs after loading the config proper, but I don't know if that distinction is really important. Maybe that's close enough that we could reuse it for this, giving users a way round the problem if there is an unknown key, without needing another option? 🤷

@Carreau

Carreau commented Jan 7, 2022

Copy link
Copy Markdown
Contributor Author

Ok, let me see. I want to test if the FLIT_ALLOW_INVALID pass the pip install -e . and python -m build. Those are my main concern. As things get more and more built in isolated envs, I'm still sort-of thinking that a value that is part of the pyproject.toml sort of make sens.

@takluyver

Copy link
Copy Markdown
Member

That's a good point. But I think a value in the pyproject.toml is perhaps not ideal, because that's controlled by the maintainers of the package, not by the people who might be trying to build it. Of course, the people downstream could patch pyproject.toml to use this option... but then they could also patch it to remove the extra keys.

@Carreau

Carreau commented Jan 7, 2022

Copy link
Copy Markdown
Contributor Author

I agree, I just don't see a reason to set/unset it outside of pyproject.toml, cause if there are extra keys then the maintainers should be aware of them.

(I actually don't see a reason to unset failing on non-existing key at all unless there are new keys and an old versions of flit...).

@pradyunsg

pradyunsg commented Jan 8, 2022

Copy link
Copy Markdown
Member

I want to test if the FLIT_ALLOW_INVALID pass the pip install -e . and python -m build.

I can confirm that it will work. :)

I use the same mechanism (environment variable) in another build backend: https://github.com/pradyunsg/sphinx-theme-builder/blob/97bd61c3a441341cd33c5af11c3264226c297b9e/src/sphinx_theme_builder/_internal/nodejs.py#L99

@takluyver

Copy link
Copy Markdown
Member

Thanks @pradyunsg . 🙂

I'm also OK with just erroring on unknown keys with no escape hatch - I think the PEP 621 support is new enough that we can probably get away with this, and we can always add an escape hatch if it turns out to be an issue. If we do want an escape hatch, the environment variable is my preference.

@takluyver

Copy link
Copy Markdown
Member

It appears that at least one package has made it to PyPI with a misspelled key - pandas_dataframe_convert==0.2 has dependancies instead of dependencies.

This is probably a prompt to get this fixed quickly. But maybe also that we do need some kind of escape hatch to let such packages be built. 😕

@Carreau

Carreau commented Jan 10, 2022 via email

Copy link
Copy Markdown
Contributor Author

@takluyver

Copy link
Copy Markdown
Member

Technically incorrect but still largely working. Given what it is, I'd guess a lot of users have pandas installed anyway.

It was only up for a matter of hours before a new release corrected the issue, so in this case we'll probably get away with breaking the older release. But it does show that real people are hitting the issue and managing to upload packages to PyPI.

@Carreau

Carreau commented Jan 11, 2022

Copy link
Copy Markdown
Contributor Author

Technically incorrect

The best kind of incorrect.

@takluyver

Copy link
Copy Markdown
Member

My current thinking is that we'll make unknown [project] keys an error for Flit 4.0. I encourage all projects to specify an upper bound (currently <4) on the version of flit_core they build with, so that we can make changes like this without breaking existing packages.

@takluyver takluyver mentioned this pull request Jan 28, 2024
4 tasks
@Carreau Carreau closed this Sep 16, 2024
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

Successfully merging this pull request may close these issues.

Consider making it possible to fail on unknown keys in [project] table

3 participants