hi there, sorry to keep posting problems! We reall...
# general
h
hi there, sorry to keep posting problems! We really appreciate all the work that you’ve put into this project but here goes: We just ran into a
ERROR: Double requirement given:
message when trying to run a recursive test of a top-level project dir: For example: pants test foo:: Imagine the structure of foo as:
Copy code
foo/
foo//tests
foo//tests/python
foo//tests/python/baz
foo//tests/python/baz/test_baz.py
foo//tests/python/baz/BUILD
foo//tests/python/bar
foo//tests/python/bar/BUILD
foo//tests/python/bar/test_bar.py
foo//src
foo//src/python
foo//src/python/baz
foo//src/python/baz/__init__.py
foo//src/python/baz/BUILD
foo//src/python/bar
foo//src/python/bar/__init__.py
foo//src/python/bar/BUILD
Now in this case, the bar module imports one dependency, say boto3==1.0 and the baz module imports a newer boto, say boto3==2.0. In our example, imagine that the baz module is a complete refactor-in-progress that doesn’t share any imports with bar. It seems that the recursive test tries to resolve all dependencies at once, and thus sees those duplicate boto3 deps and bails. In our real case it’s a py3 upgrade of a project that is using a newer numpy but I feel like the specifics aren’t as important. I realize we could just simply move baz out of the foo top-level. But it seems like pants should be resolving these dependencies separately? Is there a way to force that? I know in past versions of pants it was possible to specify
--no-test-pytest-fast
which would resolve each target’s dependencies separately. Is there something similar for pants v2?
it should be added that pants test foo/tests/python/bar:: and pants test foo/tests/python/baz:: work
h
Does your constraints file have this requirement in it twice? If so, you can use the markers like this iirc (afk)
foo==2.0 ; python_version > ‘3’
Raúl learned about this syntax a month ago
h
interesting
h
Also what do you mean by recursive test? Like pants test ::?
h
yah
we can do that on a top-level dir as well just to run all tests for a what we call a project
pants test foo::
h
Hm, I would expect the behavior to be the same whether you use that target glob or run the target/file specifically. Each file is a separate process for isolation; the glob is only shorthand for specifying multiple files at once Are you able to determine which specific file is failing? If so, try running only that file when testing things out. Lmk if that fails too
h
i’m able to run
pants test foo/tests/python/bar::
and
pants test foo/tests/python/baz::
it’s only when i try to run
pants test foo::
that it fails
h
Which suggests to me that you would have some test in another location under foo/tests that fails. Like a foo/tests/python/tofu:: Does the error message say what it's failing for? I believe it should be in the stack trace, if you're able to share that
j
./pants dependencies --transitive --type=3rdparty foo::
shows which packages have multiple requirements defined.
💯 1
I think what we are asking, is there a way for us to run
./pants test foo::
and pants to know that there are two incompatible projects under that so it runs the tests separately.
h
Well that's my concern/confusion - it should be doing that already. Unless you're using a global resolve via a constraints file, each distinct test should have its own resolve Do you have a constraints file set up?
h
we do
h
When you run these tests, do you get that warning about the constraints file *not being used?
j
So we need to add an entry for every duplicate item we find when we run
./pants dependencies --transitive --type=3rdparty foo:
, yes?
h
i think the problem is not that pytests are run separately, it’s in the dependency resolution, like if i run
pants dependencies --transitive foo
I get back the list: 3rdparty/python:boto1 3rdparty/python:boto2
h
Bump on this:
When you run these tests, do you get that warning about the constraints file not being used?
It impacts the resolve strategy, whether you’re using global resolves where you use the entire contents of
constraints.txt
, vs. multiple more granular resolves. If you’re able to, it would be useful to run with
-ldebug
and share. Totally fine if over DM
h
wait, i think you were right, it wasn’t an issue between modules, one of the modules had a direct dep on a lib that specified a different numpy dep. This is weird though, that more specific pants tests worked but not a top-level test.
👍 1
h
That is weird, and my hope is that you’re running a test file you weren’t doing when more granular. Could you do a quick check please?
Copy code
./pants filter --target-type=python_tests foo/tests:: | wc -l
./pants filter --target-type=python_tests foo/tests/subdir1:: foo/tests/subdir2:: | wc -l
h
yah sure, one sec
ok, i must have run the specific subdir tests after adding the constraints stuff (which did initially fix the problem). It turns out that one of our subdirs had a direct dep on a lib that had an alternate numpy version dep. oof, sorry about this!
h
Yay, glad it’s figured out and that indeed globbing behavior isn’t showing this bad behavior - that would have been a show stopper if true Ack though that double requirement is really confusing to figure out. Bump if the proposal in https://pantsbuild.slack.com/archives/CK79E5J2Y/p1606929205071400 sounds sensible? It’s inspired from your post and realizing “How TF would you figure this out if you’re not on Slack or know to look in the docs?”
h
yah i think when dep resolution was done natively in pex, we worked around duplicate deps in 3rdparty by doing something like: 3rdparty/python/requirements.txt:numpy>=1.15.4,<=1.16.1 And then for a project that needed a specific version of numpy:
Copy code
$ cat 3rdparty/python/BUILD.numpy
python_requirement_library(
    name = "numpy15",
    requirements = [
            "numpy==1.15.4",
    ],
)
And pex would do the right thing in selecting 1.15.4. Now it seems that for those special cases, we’ll have to add to constraints.txt something like
numpy>=1.15.4,<=1.16.1
h
Hm, although warning that won’t do quite what you want, I fear. pip uses constraints files by completely ignoring whatever the requirement string is and always using the constraint string - so if
numpy>=1.15.4,<=1.16.1
is included in your constraints file, anytime you or a transtitive dep use
numpy
, pip will substitute in the constraints value We need to support multiple constraints files, which was voted pretty highly in Q4 prioritization
Does the Numpy pinning depend entirely on whether the project is Python 2 vs. Python 3?
h
unfortunately no, some projects that are py2 need different versions of various 3rdparty libs. i know it’s less than ideal, would be nice to have everything on the same (it seems to esp hit us harder in the pydata ecosystem, numpy/pandas/scipy) but sometimes we just can’t workaround it esp if dependent upon a 3rdparty lib that has a transitive pin on a different version
👍 1
i read that pip is getting a native dep resolver soon. i also read that someone suggested it pick the min which i hope will be the case.
h
Indeed, it became the default this week for new pip! We need to upgrade Pex to consume it
h
does pip pick the min of a constraints file?
like will it pick 1.15.4 of
numpy>=1.15.4,<=1.16.1
h
pip chooses the max possible
h
ugh
h
Ugh indeed, dealing with deps is my least favorite part of Python While this wouldn’t totally solve your problems because you said it’s more than Py2 vs. Py3, this might mitigate it:
Copy code
numpy>=1.15.4,<1.16 ; python_version == '2.7'
numpy==1.16.1 ; python_version >= '3.5'
The alternative I could think of is to leave off numpy from constraints.txt, so that like v1, each project can decide exactly which
python_requirement_library
should be used, and that’s respected rather than the constraints.txt overriding everything You wouldn’t get to benefit from the global resolve for any projects using NumPy, but your other tests could still leverage it
h
yah but if we leave it out of constraints then we might run into that double requirement problem
it’s possible we were overly relying upon pex v1's correct min dep resolver and should clean up our code so that we don’t have these double requirements
h
do you know about transitive excludes via
!!
? It might make things even more confusing, but it would allow you to ignore a downstream dependency on numpy so that the call site can choose exactly which one should be used https://www.pantsbuild.org/docs/targets#dependencies-and-dependency-inference
h
oh interesting
yah, that might help, but ideally we aren’t mixing these deps up so often. but could be useful in a special case
👍 1
just got hit with this again:
elasticsearch==6.2.0 requires urllib3<1.23,>=1.21.1 but urllib3 1.25.11 was resolved
There’s no direct dep on urllib3 in this example project so I guess that something else transitively pulled it in. We seem to be in a bind here, some projects pull in a dep that has a transitive dep on say:
urllib3>1.24,urllib3<1.25
and another has a transitive like elasticsearch. We can’t increase the range in constraints.txt to
urllib3>1.21.1,urllib3<1.25
because then it would always pull in the highest and then something that required a lower range like elasticsearch would fail. I’m not quite seeing how the transitive exclude would work for this problem. Is there any way to pass an argument through to the underlying pip that is being called to activate the dependency resolver? That way we wouldn’t need to wait for pex to upgrade to pip 20.3.
h
Gr, frustrating.
Is there any way to pass an argument through to the underlying pip that is being called to activate the dependency resolver?
I don’t think so unfortunately. Also, note that the new dependency resolver is stricter than before - from what I understand, I don’t expect it to solve this problem, only to be more consistent and I suspect to have a better error message. Do you know which deps are using urllib3 and resulting in the conflict? The best I can think of would be to tweak the versions used by the direct deps until they’re in alignment - ack that this is a horrible charade
h
i would hope that the new dependency resolver in pip when given the following:
urllib3>=1.21.1,<1.25
and
urllib3==1.23
would choose
urllib3==1.23
since it satisfies both
i’ll try whacking on deps but it’s a very unsatisfying option and doesn’t really allow for great dependency flexibility long term.
h
Ah, true, that may be the case. I pinged John about this thread - he has a better understanding of what that upgrade to pip would look like. One thing that may help motivate bumping up that work is if you’d be able to do a
pip install
of the problematic deps using 20.3 and confirm it does indeed fix this problem
h
good point, i’ll test that out
Copy code
$ pip install 'git+<https://github.com/elastic/elasticsearch-py@6.2.0#egg=elasticsearch>' urllib3==1.25
Collecting elasticsearch
  Cloning <https://github.com/elastic/elasticsearch-py> (to revision 6.2.0) to /private/var/folders/vl/cj7hkk853x92cjm1k0vz0mmw0000gn/T/pip-install-e2e9q6xp/elasticsearch_bb469385dabe463ea2da93b16d31979d
Requirement already satisfied: urllib3==1.25 in /Users/nate/.pyenv/versions/3.6.9/lib/python3.6/site-packages (1.25)
ERROR: Cannot install elasticsearch==6.2.0 and urllib3==1.25 because these package versions have conflicting dependencies.

The conflict is caused by:
    The user requested urllib3==1.25
    elasticsearch 6.2.0 depends on urllib3<1.23 and >=1.21.1

To fix this you could try to:
1. loosen the range of package versions you've specified
2. remove package versions to allow pip attempt to solve the dependency conflict

ERROR: ResolutionImpossible: for help visit <https://pip.pypa.io/en/latest/user_guide/#fixing-conflicting-dependencies>
01:38:32 EST ✘ nate@Nates-MacBook-Pro:(CICD-8/pants-v2 * u+1)~/git/chartbeat
$ pip install 'git+<https://github.com/elastic/elasticsearch-py@6.2.0#egg=elasticsearch>' urllib3==1.22
Collecting elasticsearch
  Cloning <https://github.com/elastic/elasticsearch-py> (to revision 6.2.0) to /private/var/folders/vl/cj7hkk853x92cjm1k0vz0mmw0000gn/T/pip-install-4ik48g5v/elasticsearch_b9e440d5bce74e3d9639d53da62e3484
Collecting urllib3==1.22
  Using cached urllib3-1.22-py2.py3-none-any.whl (132 kB)
Building wheels for collected packages: elasticsearch
  Building wheel for elasticsearch (setup.py) ... done
  Created wheel for elasticsearch: filename=elasticsearch-6.2.0-py2.py3-none-any.whl size=71414 sha256=18a5a496cdf043215c81f9911d3d185609903ff6a6f2b739b27033f30848c3c7
  Stored in directory: /private/var/folders/vl/cj7hkk853x92cjm1k0vz0mmw0000gn/T/pip-ephem-wheel-cache-uccekbi3/wheels/5a/be/bd/47156a28570e0c035ef23b81e1d9aa1ed8ee284791ce2835f0
Successfully built elasticsearch
Installing collected packages: urllib3, elasticsearch
  Attempting uninstall: urllib3
    Found existing installation: urllib3 1.25
    Uninstalling urllib3-1.25:
      Successfully uninstalled urllib3-1.25
  Attempting uninstall: elasticsearch
    Found existing installation: elasticsearch 6.8.1
    Uninstalling elasticsearch-6.8.1:
      Successfully uninstalled elasticsearch-6.8.1
Successfully installed elasticsearch-6.2.0 urllib3-1.22
seems to work
clearly in our case, since 6.2.0 isn’t even on pypi anymore (we run our own cheeseshop to mitigate breaking unbouded deps) and 6.8.1 specifies urllib3>=1.21.0 this problem would go away with elasticsearch 6.8.1. i’ll work on upgrading that but it’s still a possible issue and looks like the dependency resolver does the correct thing
h
Wow, what a better error message. That alone would be a huge UX win for pex To clarify, it only works when using the new resolver? Or if you use those same versions with pex/Pants, it also fixes the problem?
h
let me check
ok, so i realized my test should really be urllib3 with a range larger than elasticsearch’s. Because I think what was happening in our case is we had some transitive dep giving either a larger range or gasp an unbounded range and pip was just finding the max. So my new test was with pip 20.1 (i know it’s slightly higher than the pex vendered pip but still seems reasonable). So by specifying urllib3 with a higher range, that does indeed fail (i suspect the key here is to specify urllib3 with the higher range first):
Copy code
$ pip install 'urllib3>=1.21.1,<1.25' 'git+<https://github.com/elastic/elasticsearch-py@6.2.0#egg=elasticsearch>'
Collecting urllib3<1.25,>=1.21.1
  Downloading urllib3-1.24.3-py2.py3-none-any.whl (118 kB)
     |████████████████████████████████| 118 kB 873 kB/s
Collecting elasticsearch
  Cloning <https://github.com/elastic/elasticsearch-py> (to revision 6.2.0) to /private/var/folders/vl/cj7hkk853x92cjm1k0vz0mmw0000gn/T/pip-install-tkda8gsm/elasticsearch
  Running command git clone -q <https://github.com/elastic/elasticsearch-py> /private/var/folders/vl/cj7hkk853x92cjm1k0vz0mmw0000gn/T/pip-install-tkda8gsm/elasticsearch
  Running command git checkout -q d46bbdf124a876634ec79e5bfeac0b269a6ba9e6
Building wheels for collected packages: elasticsearch
  Building wheel for elasticsearch (setup.py) ... done
  Created wheel for elasticsearch: filename=elasticsearch-6.2.0-py2.py3-none-any.whl size=71414 sha256=a6923fd48ac8fe2ec37b6712680c427c2cc7f964908310ce461abc21ffed7665
  Stored in directory: /private/var/folders/vl/cj7hkk853x92cjm1k0vz0mmw0000gn/T/pip-ephem-wheel-cache-32s1x_h_/wheels/5a/be/bd/47156a28570e0c035ef23b81e1d9aa1ed8ee284791ce2835f0
Successfully built elasticsearch
ERROR: elasticsearch 6.2.0 has requirement urllib3<1.23,>=1.21.1, but you'll have urllib3 1.24.3 which is incompatible.
Installing collected packages: urllib3, elasticsearch
Successfully installed elasticsearch-6.2.0 urllib3-1.24.3
WARNING: You are using pip version 20.0.1; however, version 20.3.1 is available.
You should consider upgrading via the '/Users/nate/.pyenv/versions/3.6.9/bin/python -m pip install --upgrade pip' command.
And now with pip 20.3.1
Copy code
$ pip install --no-cache-dir 'urllib3>=1.21.1,<1.25' 'git+<https://github.com/elastic/elasticsearch-py@6.2.0#egg=elasticsearch>'
Collecting elasticsearch
  Cloning <https://github.com/elastic/elasticsearch-py> (to revision 6.2.0) to /private/var/folders/vl/cj7hkk853x92cjm1k0vz0mmw0000gn/T/pip-install-5i7l7e9r/elasticsearch_4cf9f64cdc074837859b92a922d327a1
Collecting urllib3<1.25,>=1.21.1
  Downloading urllib3-1.22-py2.py3-none-any.whl (132 kB)
     |████████████████████████████████| 132 kB 898 kB/s
Building wheels for collected packages: elasticsearch
  Building wheel for elasticsearch (setup.py) ... done
  Created wheel for elasticsearch: filename=elasticsearch-6.2.0-py2.py3-none-any.whl size=71414 sha256=a43a95deb63864e7e36aa1232c409217dd9a057021f1c2c0abbeaa99b3f287d1
  Stored in directory: /private/var/folders/vl/cj7hkk853x92cjm1k0vz0mmw0000gn/T/pip-ephem-wheel-cache-81i0wz99/wheels/5a/be/bd/47156a28570e0c035ef23b81e1d9aa1ed8ee284791ce2835f0
Successfully built elasticsearch
Installing collected packages: urllib3, elasticsearch
Successfully installed elasticsearch-6.2.0 urllib3-1.22
👍 1
h
(i suspect the key here is to specify urllib3 with the higher range first):
Indeed, pip’s old algorithm is “first one seen” 😕 Okay, so sounds like the new pip resolver would truly make a difference here. Plus the much better error message
e
I'm late to this conversation @helpful-lunch-92084 but if you have a solution with the new Pip resolver you can make the current Pip resolver Pants uses behave like you want by modifying constraints.txt with the answers new Pip gives. Upgrading Pex to the new resolver is non-trivial since it involves apllying a few patches we have in the pantsbuild fork of pip. I'll expedite that change, but it impacts existing Pex users and a constraints edit should be a viable workaround to tide you over.
👍 2
j
@enough-analyst-54434 For our planning purposes, do you know what future version of pants the pex upgrade to the newest pip may land in? We are currently following the v2.1.x on release candidates.
e
I don't. On the Pex side I expect the release will be 2.1.23. Pex is at 2.1.21 now and the 2.1.22 release has a bunch of other fixes and improvements that need to be kept separate: https://github.com/pantsbuild/pex/issues/1111. So Pex likely timeline is 2.1.22 by Monday, Dec 14 and 2.1.23 (with option to use new Pip resolver) by Wednesday Dec 16.
j
Thank you. This is helpful.
e
The Pex Pip upgrade PR is up at https://github.com/pantsbuild/pex/pull/1133 and the release tracking issue is https://github.com/pantsbuild/pex/issues/1134.
❤️ 3