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

Prevent missing package_data in CI #6030

Closed
jpy-git opened this issue Mar 29, 2022 · 3 comments
Closed

Prevent missing package_data in CI #6030

jpy-git opened this issue Mar 29, 2022 · 3 comments
Labels
Maintenance Discussion or action around maintaining pylint or the dev workflow
Milestone

Comments

@jpy-git
Copy link
Contributor

jpy-git commented Mar 29, 2022

Current problem

This is an improvement to prevent issue like #6028 (Fixed by #6029) from occurring again.

Desired solution

2 options:

  • Don't use editable installs for testing: Editable install just uses an egg-link to link the code to site-packages and therefore simulate the package being installed. The risk is that non-python files within the code aren't automatically included in package_data and therefore, whilst present in editable install, aren't present in actual installs. This can either be via tox or switching pip install -e . with pip install .. Looking at tests.yaml it's not immediately obvious to me where the package is getting installed so hopefully a maintainer can point this out 😄

  • have a separate check to assert that certain files are within distributions:

# build wheel/source
python setup.py sdist bdist_wheel
# check file appears in them
tar --list -f dist/pylint-2.14.0.dev0.tar.gz | grep "testing_pylintrc" | wc -l
unzip -l dist/pylint-2.14.0.dev0-py3-none-any.whl | grep "testing_pylintrc" | wc -l

My preference is option 1 since option 2 requires maintaining a separate list of files to check.

Additional context

No response

@jpy-git jpy-git added the Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling label Mar 29, 2022
@Pierre-Sassoulas
Copy link
Member

We can launch the test with tox, but pytest alone is faster which is probably why we don't use tox in github action (locally you also have to --recreate or remember to reinstall if you're not using editable install). To be fair I don't think we have non python files to include in the packaging often enough to warrant a check or having to remember that we need to reinstall so the code is up to date.

There's another solution: it's to generate the default value at runtime in the testutil if the file do not exists. As those tests class are used only by downstream library maintainer this file is very seldom required so not adding it to the packaging at all will be better 99,95% of the time.

@Pierre-Sassoulas Pierre-Sassoulas added Maintenance Discussion or action around maintaining pylint or the dev workflow and removed Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling labels Mar 29, 2022
@Pierre-Sassoulas Pierre-Sassoulas added this to the 2.14.0 milestone Mar 29, 2022
@cdce8p
Copy link
Member

cdce8p commented Mar 29, 2022

I would argue against using tox. The current caching mechanism is pretty optimized, not sure the same can be archived with tox. In the end, I don't expect that to be a frequent problem. So we might also be fine doing nothing.

@jpy-git
Copy link
Contributor Author

jpy-git commented Mar 29, 2022

Yeah it's not essential, only made the issue in case you wanted extra options for checking but doesn't sound worth it 👍

There's another solution: it's to generate the default value at runtime in the testutil if the file do not exists.

We can discuss this on #6029

@jpy-git jpy-git closed this as completed Mar 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Maintenance Discussion or action around maintaining pylint or the dev workflow
Projects
None yet
Development

No branches or pull requests

3 participants