-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
[ci] validate pyproject file by json schema #6813
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really nice! I agree, I completely trust that author for anything packaging-related.
Left a question though. I'm unsure about why we'd prefer validate-pyproject-schema-store
to validate-pyproject
.
@@ -44,3 +44,8 @@ repos: | |||
- id: typos | |||
args: ["--force-exclude"] | |||
exclude: (\.gitignore$)|(^\.editorconfig$) | |||
- repo: https://github.com/henryiii/validate-pyproject-schema-store |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've read the docs of both projects, and want to be sure I understand... is the main benefit of validate-pyproject-schema-store
over validate-pyproject
that it explicitly is tied to version of the pyproject.toml
schema?
That's the impression I got from https://github.com/henryiii/validate-pyproject-schema-store/tree/main?tab=readme-ov-file#direct-usage
This also has the benefit that the version will be pinned and updated by pre-commit automatically.
Or is it that by vendoring the schema file, you avoid a network call out to the internet every time pre-commit
runs?
Given that we always build with the latest pip
, build
, and scikit-build-core
here, and that CI runs with total access to the internet and very rare network interruptions, I think we want to always keep up with the latest pyproject.toml
schema. And so think it would be better to use validate-pyproject
and ensure every CI run gets the latest pyproject.toml
schema.
What do you think? Did I misunderstand?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Initially I thought that it's good idea to pin the schema version and have a tight control over its updates. But I don't mind having the latest version always. Especially given that our dependabot doesn't update pre-commit hooks. I'll switch to validate-pyproject
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, I just checked with [tool.ruff.lint.select] = [..., "non-existing"]
deep intentional error. And validate-pyproject
validated successfully that file while validate-pyproject-schema-store
failed with the following error:
Validate pyproject.toml..................................................Failed
- hook id: validate-pyproject
- exit code: 1
Error: `tool.ruff.lint` cannot be validated by any definition:
Given this difference in behavior of these two tools, do you still want to replace validate-pyproject-schema-store
with validate-pyproject
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh interesting. I still don't really understand the difference between these tools but... seeing the things validate-pyproject-schema-store
is catching, I'm ok with trying it. Thanks for considering and testing validate-pyproject
.
Let's merge this as-is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TBH, me too. I thought that validate-pyproject-schema-store
is just a something like a caching wrapper for schemas, but seems that differences are bigger than I thought. OK, at least I can see all partial schemas here in the repo: https://github.com/henryiii/validate-pyproject-schema-store/tree/main/src/validate_pyproject_schema_store/resources and it makes me feel that all sections of the file are really validated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, seems that behavior is documented:
According to PEP 518, tools can define their own configuration inside pyproject.toml by using custom subtables under tool.
In validate-pyproject, schemas for these subtables can be specified via Plugins. The following subtables are defined by built-in plugins (i.e. plugins that are included in the default distribution of validate-pyproject):
https://validate-pyproject.readthedocs.io/en/latest/schemas.html#tool-table
And I can see only tool.setuptools table right after that passage.
UPD: More strong evidence:
Since different tools allow different configurations, it would be impractical to try to create schemas for all of them inside the same project. Instead, validate-pyproject allows Plugins to provide extra JSON Schemas, against which tool subtables can be checked.
https://validate-pyproject.readthedocs.io/en/latest/dev-guide.html#how-it-works
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah ok, that explains it! Then yes, this makes me even more confident in using validate-pyproject-schema-store
.
Originally, I wanted to add a linter for TOML files in our repo. But later I found these two great projects:
Please don't be afraid of few stars for these projects' repos. Just check author's profile. I believe this project is simply underrated one.
Json schema: https://github.com/SchemaStore/schemastore/blob/master/src/schemas/json/pyproject.json
Here is an example of failing job with an intentional mistake in
classifierss
(note doubles
at the end) section spelling.