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

Nightly builds fail because main branch of pylint and astroid are incompatible with each other #347

Closed
carlio opened this issue Jan 5, 2022 · 7 comments
Assignees
Labels

Comments

@carlio
Copy link
Collaborator

carlio commented Jan 5, 2022

I added nightly builds to test compatibility with bleeding edge dependencies (to try to catch things early - see issue comment here)

However after a recent update, the main branch of astroid's version is 2.10-dev0 but pylint main branch has a version restriction of <2.10 for astroid. This means the two cannot be installed at the same time - see the output of https://github.com/PyCQA/pylint-django/actions/runs/1656157819 for example.

This issue is to figure out what to do:

  • abandon bleeding edge testing? The main branches of astroid and pylint are considered dev versions rather than stable releases
  • force pip to install even if version dependency restrictions don't quite match?
  • something else?
@carlio carlio added the todo label Jan 5, 2022
@carlio carlio self-assigned this Jan 5, 2022
@carlio
Copy link
Collaborator Author

carlio commented Jan 11, 2022

I'm starting to think that the correct solution is to pin pylint and also astroid for pylint-django to a 'known good' range rather than simply a minimum.

#105 makes me think that the open-ended dependencies can cause strange conflicts, especially since astroid and pylint have different release cadences. I'm not sure if that was always the case but it seems to be true now.

Therefore to do:

1 - set specific ranges for pylint and astroid in setup.py as 'known compatible versions'
2 - create a separate job (and schedule) to test against latest released pylint+astroid to find problems before users do.

@DanielNoord
Copy link

The correct solution is for us to rename the dev version of astroid 😄

I think @Pierre-Sassoulas might have made a mistake by setting __version__ to 2.10 in pylint-dev/astroid@e5d33e6
In fact there is a commit that sets it to 2.9.4 here: pylint-dev/astroid@4fd3d34 but this was later overwritten I guess? (The commit history is a bit weird here...)

@Pierre-Sassoulas Could you fix this by changing the working __version__ of astroid to 2.9.4-dev0 again? I'm also running into this issue with local editable installs, but didn't open a issue yet.

@carlio In the future feel free to ping me for upstream related issues with either pylint or astroid. I don't follow this repo and saw this by chance, but I'm happy to help out whenever I can!

@Pierre-Sassoulas
Copy link
Member

I think the solution is to set the pylint version to bleeding edge and let pylint set his version of astroid.

The current dev version of astroid is 2.10.0-dev0 on main, and 2.9.4-dev0 on the 2.9 branch. (The peculiar history is required so pre-commit understand that 2.9.3 is a released tag and pre-commit autoupdate works).

If you want to set astroid yourself you could use the latest version on the 2.9 branch but I strongly advise against that because only pylint knows what version of astroid it's compatible with. There will always be a time when bleeding edge astroid won't be compatible with bleeding edge pylint, we often have a branch that is compatible with bleeding edge astroid before it's released but you don't want to have to care about that.

@DanielNoord
Copy link

There will always be a time when bleeding edge astroid won't be compatible with bleeding edge pylint, we often have a branch that is compatible with bleeding edge astroid before it's released but you don't want to have to care about that.

Ah I didn't think about this. Although I wonder if we can do it the other way around: allow bleeding edge astroid on pylint and disallow when we do have an incompatible version on pylint main. These errors with editable installs are really quite annoying. (This discussion should probably take place on the pylint repo though).

@carlio
Copy link
Collaborator Author

carlio commented Jan 12, 2022

I think my original approach still makes sense for what I wanted to implement : a nightly canary build using latest stable dependencies so that we get a notification if any problems pop up without relying on users having broken builds and reporting an issue.

There's no particular need to test against bleeding edge pylint+astroid. The behaviour was ported from the Travis config and I think it was made when the assumption was that pylint@master and astroid@master (it was still called that then!) were stable.

cc @atodorov , let me know if you think my plan of nightly build using latest PyPI versions is sufficient or if you think we should keep testing against bleeding edge, given the conversation above

@Pierre-Sassoulas
Copy link
Member

Bleeding edge pylint is supposed to be working, and it should never have an issue with the version of astroid (we're using only released version of astroid in pylint's main branch). Problems will arise if you're trying to set astroid version manually. Having a test using bleeding edge pylint would be really interesting.. for pylint-django as well as for pylint itself. I don't know if launching the pylint-django test suite from pylint is feasible but that would be ideal.

@DanielNoord
Copy link

@carlio Just to let you know, we just merged pylint-dev/pylint#5689.

This should allow installation of the latest main version of astroid for the latest main version of pylint. Please let me know if we can do anything else!

@carlio carlio closed this as completed Feb 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants