https://pantsbuild.org/ logo
Join Slack
Powered by
So I’ve been looking into why I cannot install a p...
# general
a
So I’ve been looking into why I cannot install a particular pylint version… I’m trying to use 
2.4.15
, which needs python 3.7 (which is what our repo is supposed to use). When I try to generate a lockfile it says:
Copy code
ERROR: Could not find a version that satisfies the requirement pylint>=2.14.5
ERROR: No matching distribution found for pylint>=2.14.5
If I look in the logs, it says it’s using 3.7.4 to bootstrap the env, and if I check in the sandbox, the pex file is python 3.7.4. But if I run 
__run.sh
, it gets changed to 3.6.8 (this was a pyenv artifact). If I use the pex from the sandbox folder to get pylint, it works. If I run 
__run.sh
, it obviously stops working. There’s also nothing in the log file mentioned in the command line printed. How can I debug/fix this, except by uninstalling any 
<3.6
I have?
If I try to run it with PEX verbose I get the hilarious error that the argument list is too long…
Copy code
Failed to spawn a job for /Users/cris.birzan/.pyenv/versions/3.7.4/envs/crpython-3.7/bin/python: [Errno 7] Argument list too long: '/Users/cris.birzan/.cache/pants/named_caches/pex_root/venvs/c9e55cc98846b062ba9676b3c1a5214512602544/9e8a8413acb01e6e9c00d46a84284b42001f4885/bin/python'
e
@ancient-france-42909 can you provide the full command line + output of 
./pants generate-lockfiles --resolve=pylint -ldebug
a
e
This is the same issue we solved yesterday: 
"./pex", "lock", "create", ..., "--interpreter-constraint", "CPython==3.7.*"
You need to adjust your interpreter-constraint from 
CPython==3.7.*
CPython>=3.7.2,<3.8
This is because pylint 2.14.5 requires Python 
>=3.7.2
a
I seriously doubt that’s the case.
e
And its because we're creating a universal lock that should work on any machine and not just yours.
I seriously doubt that’s the case.
You seriously doubt what is the case?
a
First of all, my interpreter constraint is 
>=3.7.4
e
That is definitely not what the logs say.
Can you provide your pants.toml?
a
Untitled.txt
Untitled.txt
e
Are there any other relevant PANTS_ env vars in your environment or a ~/.pants.rc file on your machine?
Aha, I see this in another log line: 
--interpreter-constraint CPython==3.7.* --interpreter-constraint CPython>=3.7.4
- that implies you have ICs defined also in BUILD files somewhere.
Can you grep for those?
a
Okay, so. It seems that was the issue, in a BUILD file. But, that’s broken. 
>=3.7.4
is more specific than 
==3.7.*
.
So it should’ve been considered equivalent to 
~=3.7.4
e
If you want to distill that down to an issue, you may be right there. I'm trying to keep my focus small since this has been pretty hard to grok from a support end though. Does the 2.14.5 (temporary) solution lead to a pylint fix for you?
a
Well, ironically, I need to wait to generate my mypy lockfile
e
FWIW, passing more than 1 IC means OR them to Pex. Pants may or may not be incorrect in passing two ICs to Pex there.
Ok, let me know when the ironies and etc are passed and you have something for me to look at.
a
It’s not my fault it takes 20 minutes to generate lockfiles.
e
The Pex docs fwiw:
Copy code
--interpreter-constraint INTERPRETER_CONSTRAINT
                        Constrain the selected Python interpreter. Specify with Requirement-style syntax, e.g. "CPython>=2.7,<3" (A CPython interpreter with version >=2.7 AND version <3), ">=2.7,<3" (Any Python interpreter with version >=2.7 AND version <3) or "PyPy" (A
                        PyPy interpreter of any version). This argument may be repeated multiple times to OR the constraints. Try `PEX_TOOLS=1 /home/jsirois/.venv/pex/bin/python3 /home/jsirois/.venv/pex/bin/pex interpreter --verbose --indent 4` to find the exact
                        interpreter constraints of /home/jsirois/.venv/pex/bin/python3 and `PEX_TOOLS=1 /home/jsirois/.venv/pex/bin/pex interpreter --all --verbose --indent 4` to find out the interpreter constraints of all Python interpreters on the $PATH. (default: [])
So Pants may be doing the wrong thing here for sure.
a
My other question is, what’s with the arg list too long?
e
Not sure about that. If you agree to keep focus on the pylint, I'd like to handle that 1st,
It’s not my fault it takes 20 minutes to generate lockfiles.
But it is you coming across a bit whiny. I realize you're not happy. I hope you realize I'm working hard here to help you.
a
So, I tried to run it, it took 4 minutes to complain that the bandit lockfile is out of date. 🙂
e
That's a bit surprising since your command line restricts to just 
--resolve=pylint
Can you provide the full command line plus complaint?
a
Oh, sorry. I thought I sid that it could generate a lockfile after I removed the 
==3.7.*
constraint. (Which, in my defense, was added last night) This is for trying to lint, to see if this actually fixes the namespaces issue
e
Ok. I'm still a bit lost, but let me know if the pylint issue is fixed or fails with full command line + `-ldebug`and full output.
a
Which pylint issue?
e
Sorry, I'm just aware of the namespace package pylint issue.
That's where I got involved yesterday.
a
Yeah, the lockfile was generated with 2.4.15, I’m trying to figure out if that fixes the original issue, had to change the CI to not run typechecks since those would fail because of the outdated lockfile. Fixed bandit and isort for lint, running it both locally and on the CI to see which will finish first. My laptop (because apple) overheats after about 5 minutes of building virtualenvs and get throttled to like 30%, so, yeah
e
Ok.
a
Okay, so, it didn’t fix the issue. 😞
Let me see if it’s using the correct astroid, there’s a duplicated ticket says that this should be fixed in 2.15
e
Ok. Yesterday I tried to setup what I though was an example repo locally to simulate your issue and I could not hit it. If you could do something similar, that would definitely be the easiest way to help you figure this out - for me. That is a good bit of work for you though.
Basically, having a shareable repro case would be gold.
a
Yeah, I couldn’t get a simple reproducible case either.
e
Frustrating.
My hunch is this has nothing to do with ns packages. Since there are no import errors and the pylint error isn't nail-in-the-coffin about ns packages. It's about 
lib
missing in a module. That could certainly mean ns package issue, but maybe it means something more subtle.
I am not familiar at all with pylint except for my local experiment yesterday.
a
The astroid ticket (and pylint one) does suggest that’s the case.
e
Pants / Pex / Pip understand vcs so you could try HEAD with 
"pylint @ git+<https://github.com/PyCQA/pylint/commit/2fafadc8aca1831c594cafd35a46b691eaa3e357>"
as your custom pylint version.
a
Yeah, I’ll need to get the astroid commit, though.
e
a
Though, that was released in 3 versions so far it, seems.
e
So if the astroid fix is in any of `astroid>=2.12.2,<=2.14.0-dev0`you should be good with just the pylint vcs requirement.
a
yeah
e
Ok, then my original suggestion should give you a fighting chance.
What version of astrois does the current 2.14.5 lock pick?
a
2.11.7
e
Ok, excellent. This has a glimmer of hope then using the vcs requirement.
a
Is that syntax supposed to work, or do I need the normal pip stuff?
Copy code
WARNING: Discarding git+<https://github.com/PyCQA/pylint/commit/2fafadc8aca1831c594cafd35a46b691eaa3e357>. Command errored out with exit status 128: git clone -q <https://github.com/PyCQA/pylint/commit/2fafadc8aca1831c594cafd35a46b691eaa3e357> /Users/cris.birzan@tessian.com/.cache/pants/named_caches/pex_root/pip_cache/.tmp/pip-download-pmbr7jx7/pylint_b7e500279f75437f91a114cebfd68968 Check the logs for full command output.
It seems it needs a git URL
pylint@git+<https://github.com/PyCQA/pylint.git@2fafadc8aca1831c594cafd35a46b691eaa3e357>
worked.
Or at least didn’t error out yet 🙂
e
Yeah, that's why I put the @ in ther. You can do it without pylint@, but you need to add 
#egg=pylint
to the end of the vcs URL if so.
The @ style is PEP'd, the #egg= style is Pip proprietary.
a
Anyway, will report back in 20-30 minutes if it worked.
e
Excellent, thanks.
a
It is using astroid 
2.12.2
, but I still get the errors 😞 But, I’m gonna try harder to find out how to reproduce this. For example, in the sandbox, if I run pylint on all the files, I get the error. If I just run it on the ones that have an error, I don’t. So I’ll just start trying to add files and see when it errors out
e
Ok. Yeah, slogging through that and getting a on/off repro would be excellent.
a
OH
audit/lib/tests/unit/test_validate_event.py
common/audit/src/common/audit/__init__.py
The combination of two modules with the same name confuses it.
Or, well, the second audit is a module, the first is just a namespace.
e
I'm not following completely, but it sounds like you're on to something.
a
We have a module named 
audit
in another namespace, and that’s what’s causing the issue.
I didn’t pay too much attention to the PR that was supposed to fix this, but it was related to not being able to find namespaces, so they’d try to one part at a time? I’ll go back and read it in a bit. However, it’s still weird, since 
common
is not in the path, nor is 
common/audit/src/common
so you could find something called 
audit
e
I would need to see the sys.path + the 2 file paths passed to pylint in your repro case to make more sense of this. But if you're on the way to figuring this out on your own, there is no need.
It's also surprising there is an 
__init__.py
at all (to me), I thought this was all PEP-420 implicit ns packages / no dunder-inits.
a
Okay, I guess this is a different issue, 
is_namespace
from astroid returns true.
But, now I have a simple example to work with. And, yeah, it’s not really pants’ fault.
Would it be possible to split the ‘shards’ pylint is run based on the roots, btw?
e
Definitley. The rule system is pretty flexible. I am not familiar with that rule set though. Let me see if there is an option for this. We do it for other rule sets I'm pretty sure.
It looks like that is not supported today. It partitions, but just based on resolve if you use multiple resolves and by interpreter constraint if you use disjoint sets of those across your codebase. So it would take feature-work to add support for partitioning in other ways.
You can always ad-hoc partition anything using Pants by using tags on targets though.
Just a sec for links to that concept.
a
Yeah, but we’d need to run them separately.
e
That can work, but there is upkeep of tagged targets and you have to run multiple Pants commands seperately, one per set of tags you want to operate on in isolation. So not ideal, but sometimes enough.
Exactly. Sometimes fine if CI is all your really care about and it won't affect devs.
a
I mean, we plan to move to a system where we run every lib/app’s tests/checks in a separate container, so it won’t be a huge deal for much longer (2 weeks?)
I think I’ll try to figure out where the bug is in pylint/astroid and try to get that fixed.
e
Sounds good.
a
We already use tagging, but just for filtering, and I’m not 100% sure from there how we could split the specs by tags, I guess?
e
I'm not sure I follow. Tags do in fact filter any specs given on the CLI, so yes I think.
a
I mean, we can filter, but we cannot group by tags, so we’d need to get all the tags and manually iterate. At that point, we might as well just iterate over the sources roots.
e
Ah, gotcha. Yeah, sounds about right.
a
I managed to get a simple example reproducing this issue, I’ll make a ticket with pylint tomorrow. Thank you very much for the help, and sorry for my snarky remarks, most of this was my fault, but the very slow feedback loop and generally annoying debugging things did annoy me 😞
e
No worries George. Thanks for sticking through this and contributing back to Pylint.
a
I have a feeling that because I picked up this upgrade ticket, I will be doing that a bit 😛
Maybe, once I’m done fixing my problems, I’ll be able to contribute something useful.
This was painful to track down, and still not 100% sure this is the correct solution, though it only breaks one test that… well, I don’t think is needed (but still shouldn’t fail) and some tests that rely on the name of modules.
e
Hrm. I'm glad you hunted down some error there, but it is not the original error you reported in another thread, which was:
Copy code
audit/lib/tests/unit/test_validate_event.py:73:24: E1101: Module 'audit' has no 'lib' member (no-member)
I.E.: E1101 and not E0401 or E0611
a
It’s because I’m importing 
audit.lib
there, vs 
import audit; audit.lib
Or something like that, but the cause is the same, the fact that the names of the modules are wrong when you specify them on the command line.
e
AHA! So you solved that other thread! That's excellent.
It would be good form to close out that thread. I, for one, was still hanging on the outcome / resolution.
a
I found the cause and I’m making a PR right now. I only had like 2 hours today to do any actual work, Mondays seem to be the day everyone wants to have meetings in my company.
e
Ok, maybe not solved then. Ok, great. I had actually thought that might be the issue for E1101 and concocted that scenario in my example repo but could not get E1101 to trigger.
7 Views
Open in Slack
PreviousNext
Powered by