howdy, we’re in the process of migrating from 1.25...
# general
h
howdy, we’re in the process of migrating from 1.25 to 2.1.0 (i know such a jump is less than ideal…) anyways, trying to run a test on specific target (which is a large target) results in a max recursion limit exceeded I tried hacking on sys.setrecursionlimit and found for our use case 1021 was the min that would allow the test to succeed. We can try breaking that target up, but the app it’s testing is kind of a monolith so i don’t know how successful that would be. Does pants have a runtime way to increasing this limit?
if it’s helpful, here’s the stacktrace:
Copy code
Engine traceback:
  in select
  in pants.core.goals.test.run_tests
  in pants.core.goals.test.enrich_test_result (tests/python/test_djangofly/cbpsso/authentication_update_tests.py:../djangofly_apps_tests)
  in pants.backend.python.goals.pytest_runner.run_python_test (tests/python/test_djangofly/cbpsso/authentication_update_tests.py:../djangofly_apps_tests)
  in pants.backend.python.goals.pytest_runner.setup_pytest_for_target
  in pants.engine.internals.graph.transitive_targets
Traceback (most recent call last):
  File "/Users/nate/.cache/pants/setup/bootstrap-Darwin-x86_64/2.1.0_py36/lib/python3.6/site-packages/pants/engine/internals/native.py", line 65, in generator_send
    res = func.send(arg)
  File "/Users/nate/.cache/pants/setup/bootstrap-Darwin-x86_64/2.1.0_py36/lib/python3.6/site-packages/pants/engine/internals/graph.py", line 338, in transitive_targets
    _detect_cycles(tuple(t.address for t in roots_as_targets), dependency_mapping)
  File "/Users/nate/.cache/pants/setup/bootstrap-Darwin-x86_64/2.1.0_py36/lib/python3.6/site-packages/pants/engine/internals/graph.py", line 291, in _detect_cycles
    visit(root)
  File "/Users/nate/.cache/pants/setup/bootstrap-Darwin-x86_64/2.1.0_py36/lib/python3.6/site-packages/pants/engine/internals/graph.py", line 286, in visit
    visit(dep_address)
  File "/Users/nate/.cache/pants/setup/bootstrap-Darwin-x86_64/2.1.0_py36/lib/python3.6/site-packages/pants/engine/internals/graph.py", line 286, in visit
    visit(dep_address)
  File "/Users/nate/.cache/pants/setup/bootstrap-Darwin-x86_64/2.1.0_py36/lib/python3.6/site-packages/pants/engine/internals/graph.py", line 286, in visit
    visit(dep_address)
  [Previous line repeated 1005 more times]
  File "/Users/nate/.cache/pants/setup/bootstrap-Darwin-x86_64/2.1.0_py36/lib/python3.6/site-packages/pants/engine/internals/graph.py", line 288, in visit
    path_stack.remove(address)
  File "/Users/nate/.cache/pants/setup/bootstrap-Darwin-x86_64/2.1.0_py36/lib/python3.6/_collections_abc.py", line 582, in remove
    if value not in self:
  File "/Users/nate/.cache/pants/setup/bootstrap-Darwin-x86_64/2.1.0_py36/lib/python3.6/site-packages/pants/util/ordered_set.py", line 57, in __contains__
    return key in self._items
RecursionError: maximum recursion depth exceeded
h
thanks for posting the stacktrace
h
i hacked a recursion limit here to get it to work:
Copy code
$ cat ~/.cache/pants/setup/bootstrap-Darwin-x86_64/2.1.0_py36/bin/pants
#!/Users/nate/.cache/pants/setup/bootstrap-Darwin-x86_64/pants.CseQil/install/bin/python3.6
# -*- coding: utf-8 -*-
import re
import sys
from pants.bin.pants_loader import main
sys.setrecursionlimit(1021)
if __name__ == '__main__':
    sys.argv[0] = re.sub(r'(-script\.pyw|\.exe)?$', '', sys.argv[0])
    sys.exit(main())
h
@witty-crayon-22786 and @hundreds-father-404 are more familiar with this code than I am
it looks like this is happening in some code where we implement a depth-first search as part of dependency inference, which certainly could blow up the default python recursion limit
anyway at the moment, we don't have a mechanism to set
sys.setrecursionlimit
in pants
w
so… would you be able to do that in a
conftest.py
…? that would allow you to do it once for the entire repo, for example EDIT: Oh… but native support for pulling in
conftest.py
automatically didn’t land until the 2.0.0.beta series i think.
unrelated to your question, but: have you seen https://www.pantsbuild.org/docs/upgrade-tips ?
h
yah, i know, one at a time 🙂 it’s hard to keep up! i’ve been working with another dev who’s primarily been working on this, @jolly-midnight-72759. i just hopped back onto the project and came across this error. i think he’s been following most of the upgrade tips tho. interesting about conftest.py, since we’re testing 2.1.0 now that might work
w
yea… they should always have worked if they were depended-on manually, but >2.0.0.beta should automatically infer dependencies on them, which makes them a bit less awkward to use widely
h
Which you can confirm by running
./pants dependencies path/to/my_test.py
and confirming you see something like
path/to/conftest.py
in the result. (See https://www.pantsbuild.org/docs/python-test-goal#conftestpy) But Stu, we should still probably figure out a principled fix to the recursion limit, like possibly rewriting that code
👍 1
w
…oh. hah. i didn’t actually look at the stacktrace, sorry. assumed it was in the code under test. yea, we should definitely have fixed that.
h
Wait but also I’m not sure conftest.py would even fix this? The error is happening in Pants’s setup, not in the actual Pytest subprocess. conftest.py would only impact the test
coke
w
sorry: @helpful-lunch-92084: yes, we should definitely fix that. sorry for the trouble
i’ll get something out today.
❤️ 2
h
oh wow thanks!
w
it might be another day or two until it goes into a release… in the mean time, does your repository have any Pants plugins?
register.py
files?
j
Yes
w
if so, can move your sys.setrecursionlimit hack there… if not, might be able to make a tiny one to home the hack until we get the fix out.
j
@witty-crayon-22786 is there an existing issue for this?
w
there is not
j
Want us to do the honors?
w
i was planning to post a PR rather than an issue: but yea, please feel free
j
👍🏽
I've confirmed that putting @helpful-lunch-92084's fix into a plugin's
register.py
resolves the issue.
👍 2
h
Hey Nate and Raúl, both 2.0.1rc2 and 2.1.1rc0 are in the release process to be released today. Both include the recursion fix Thanks again for the report!
👖 1
🏗️ 1