Test DebugAdapter requests, and fix issues #17678

thejcannon · 2022-11-29T20:05:22Z

Added new test-code which tests the DebugAdapterRequest by running it without --wait-for-client. This wouldn't catch issues like #17540 which require a client, but would catches issues like #17672 and #17692.

Fixes:

Bad debugpy ICs
Untracked issue where the process would always return 0
The config wouldn't be used if provided
Removes the reliance on importlib_metadata to load the entrypoint, instead re-enters the already executing pex in-process

jsirois · 2022-11-29T20:24:26Z

This was a ~WAG on my part. In the absence of a test it would be great if @CJTurpie could at least confirm by applying the patch in ~/.cache/pants/setup/bootstrap-*/2.16.*/... and giving it a spin. It would be especially great to do this in the absence of a test prior to cherry-picking.

I had and continue to have ~0 context besides this fixed line of code and the base class method it calls, I read and understood no more.

thejcannon · 2022-11-29T20:29:26Z

Certainly specifying the ICs to be compatible with the field set/python default isn't a bad idea. But yes I agree we have no idea if this does fix #17672 without confirmation.

@jsirois if you can imagine a scenario where this would an issue (maybe not the linked issue) I can add a test. As it stands, I'm struggling to piece it all together on how to make a failing test.

jsirois · 2022-11-29T20:30:49Z

@thejcannon I can't see any issue in general but I have no clue about how debugpy works at all. I'd stringly suggest just slowing down and nailing this solid.

thejcannon · 2022-11-29T20:45:39Z

Ah so I figured out how to reproduce by forcing a test's ICs to 3.9, and ensuring Pants itself was running on 3.8 and I can reproduce the error.

With as many to_pex_requests there are in the codebase, I'm surprised more don't pass ICs.

CJTurpie · 2022-11-30T14:16:36Z

I can confirm that this has fixed #17672 but has thrown up another error after I try to connect VSCode to the debugger.

Traceback (most recent call last):
  File "/Users/carlturpie/.cache/pants/named_caches/pex_root/venvs/s/8e7d5b6b/venv/lib/python3.9/site-packages/importlib_metadata/__init__.py", line 288, in __getitem__
    return next(iter(self.select(name=name)))
StopIteration

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/private/var/folders/vt/dcxqbnwx731dttw2b1ngmsc00000gn/T/pants-sandbox-tri6cg/./.cache/pex_root/venvs/4e8d5b27a5a9f5b3632585044ba3ea800870f4e4/b650ff237ac9fb4381cecc1688ad035a2365dc36/pex", line 248, in <module>
    runpy.run_module(module_name, run_name="__main__", alter_sys=True)
  File "/Users/carlturpie/.pyenv/versions/3.9.15/lib/python3.9/runpy.py", line 225, in run_module
    return _run_module_code(code, init_globals, run_name, mod_spec)
  File "/Users/carlturpie/.pyenv/versions/3.9.15/lib/python3.9/runpy.py", line 97, in _run_module_code
    _run_code(code, mod_globals, init_globals,
  File "/Users/carlturpie/.pyenv/versions/3.9.15/lib/python3.9/runpy.py", line 87, in _run_code
    exec(code, run_globals)
  File "/Users/carlturpie/.cache/pants/named_caches/pex_root/venvs/s/8e7d5b6b/venv/lib/python3.9/site-packages/debugpy/__main__.py", line 45, in <module>
    cli.main()
  File "/Users/carlturpie/.cache/pants/named_caches/pex_root/venvs/s/8e7d5b6b/venv/lib/python3.9/site-packages/debugpy/server/cli.py", line 444, in main
    run()
  File "/Users/carlturpie/.cache/pants/named_caches/pex_root/venvs/s/8e7d5b6b/venv/lib/python3.9/site-packages/debugpy/server/cli.py", line 344, in run_code
    eval(code, {})
  File "<string>", line 1, in <module>
  File "/Users/carlturpie/.cache/pants/named_caches/pex_root/venvs/s/8e7d5b6b/venv/lib/python3.9/site-packages/importlib_metadata/__init__.py", line 290, in __getitem__
    raise KeyError(name)
KeyError: 'console_scripts'

I think this is a separate issue though so I can open another issue if that's the case?

thejcannon · 2022-11-30T14:23:09Z

Yeah it's a separate issue 😔

One step forward though!

thejcannon · 2022-11-30T18:43:50Z

I just had a great idea on how to test this. Hold on...

thejcannon · 2022-11-30T18:55:05Z

Ah bad merge. Will force push to clean it up since none of the code has been reviewed yet

thejcannon · 2022-11-30T19:00:12Z

OK @jsirois now there are automated tests that caught the issue (and some others yay). I verified by running with the new test code but main code for everything else and saw a failure that looked exactly like the linked issue.

thejcannon · 2022-11-30T19:52:22Z

This is odd. src/python/pants/backend/python/goals/pytest_runner_integration_test.py::test_extra_output passes locally 😵

thejcannon · 2022-11-30T20:32:33Z

@CJTurpie excellent news! #17692 is reproducing in CI with the new tests! 👍

thejcannon · 2022-12-01T17:48:52Z

Wow this exploded (in a good way). I'll be making some new PRs split out of this one as prereqs.

Flip how we execute debugpy in run #17699
Run python sources with a VenvPex #17700 (which speeds up all ./pants run file.py, 🎉 )

thejcannon · 2022-12-07T15:49:25Z

src/python/pants/backend/python/subsystems/debugpy.py

@@ -27,7 +27,7 @@ class DebugPy(PythonToolBase):
    default_main = EntryPoint("debugpy")

    register_interpreter_constraints = True
-    default_interpreter_constraints = ["CPython>=3.7,<3.11"]
+    default_interpreter_constraints = ["CPython>=3.7"]


This matches the upstream ICs on the package (and the old value was causing a conflicting ICs error)

thejcannon · 2022-12-07T15:51:56Z

OK this should be good. I'm really glad we're testing things now.

I also opened fabioz/PyDev.Debugger#243 which should simplify the run case when we can leverage it.

jsirois · 2022-12-07T18:04:27Z

I'm going to hold back on review until this thing is green.

thejcannon · 2022-12-07T18:29:47Z

Al flakes, review away!

src/python/pants/backend/python/goals/run_python_source.py

jsirois · 2022-12-07T18:55:22Z

OK this should be good. I'm really glad we're testing things now.

We is you! I'm glad too. Thanks for slowing down and doing the right thing here.

thejcannon · 2022-12-07T19:22:00Z

OK this should be good. I'm really glad we're testing things now.

We is you! I'm glad too. Thanks for slowing down and doing the right thing here.

The trick was realizing I could run the debug code by simply monkeypatching the --wait-for-client away.

src/python/pants/backend/python/goals/run_helper.py

Fixes pantsbuild#17672 and fixes pantsbuild#17692 Added new test-code which tests the `DebugAdapterRequest` by running it without `--wait-for-client`. This wouldn't catch issues like pantsbuild#17540 which require a client, but would catches issues like pantsbuild#17672 and pantsbuild#17692. Fixes: - Bad debugpy ICs - Untracked issue where the process would always return 0 - The config wouldn't be used if provided - Removes the reliance on `importlib_metadata` to load the entrypoint, instead re-enters the already executing pex in-process

…17768) Fixes #17672 and fixes #17692 Added new test-code which tests the `DebugAdapterRequest` by running it without `--wait-for-client`. This wouldn't catch issues like #17540 which require a client, but would catches issues like #17672 and #17692. Fixes: - Bad debugpy ICs - Untracked issue where the process would always return 0 - The config wouldn't be used if provided - Removes the reliance on `importlib_metadata` to load the entrypoint, instead re-enters the already executing pex in-process

thejcannon added the category:bugfix Bug fixes for released features label Nov 29, 2022

thejcannon added this to the 2.14.x milestone Nov 29, 2022

thejcannon requested review from jsirois and Eric-Arellano November 29, 2022 20:05

thejcannon added the needs-cherrypick label Nov 29, 2022

CJTurpie mentioned this pull request Nov 30, 2022

Running tests with --debug-adapter fails #17692

Closed

fix em

94bdd3b

thejcannon force-pushed the debugpy_ics branch from a0187b9 to 94bdd3b Compare November 30, 2022 18:55

thejcannon changed the title ~~Pass ICs to debugpy PEX creation~~ Test DebugAdapter requests, and fix issues Nov 30, 2022

thejcannon requested a review from stuhood November 30, 2022 19:00

do some logging

6ab43be

thejcannon added 7 commits December 1, 2022 12:00

test changes

e6e9375

Merge branch 'main' into debugpy_ics

1e755f0

basically there

0d4ffdf

Merge branch 'main' into debugpy_ics

f88a4a7

check dap

a17dae3

Merge branch 'main' into debugpy_ics

d07da36

it works!

fa1566d

thejcannon commented Dec 7, 2022

View reviewed changes

lint

8659dfa

jsirois approved these changes Dec 7, 2022

View reviewed changes

src/python/pants/backend/python/goals/run_python_source.py Show resolved Hide resolved

thejcannon added 2 commits December 7, 2022 13:28

comment

0ea9a70

Merge branch 'main' into debugpy_ics

d66ecb9

thejcannon commented Dec 8, 2022

View reviewed changes

src/python/pants/backend/python/goals/run_helper.py Show resolved Hide resolved

Update src/python/pants/backend/python/goals/run_helper.py

f6eb271

thejcannon commented Dec 8, 2022

View reviewed changes

src/python/pants/backend/python/goals/run_helper.py Outdated Show resolved Hide resolved

Update src/python/pants/backend/python/goals/run_helper.py

49b0a5f

thejcannon merged commit 22c83f1 into pantsbuild:main Dec 8, 2022

thejcannon deleted the debugpy_ics branch December 8, 2022 14:53

thejcannon mentioned this pull request Dec 9, 2022

Run python sources with a VenvPex (Cherry-pick of #17700) #17761

Merged

thejcannon modified the milestones: 2.14.x, 2.15.x Dec 9, 2022

thejcannon removed the needs-cherrypick label Dec 30, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Test DebugAdapter requests, and fix issues #17678

Test DebugAdapter requests, and fix issues #17678

thejcannon commented Nov 29, 2022 •

edited

Loading

jsirois commented Nov 29, 2022

thejcannon commented Nov 29, 2022

jsirois commented Nov 29, 2022

thejcannon commented Nov 29, 2022

CJTurpie commented Nov 30, 2022

thejcannon commented Nov 30, 2022

thejcannon commented Nov 30, 2022

thejcannon commented Nov 30, 2022

thejcannon commented Nov 30, 2022

thejcannon commented Nov 30, 2022 •

edited

Loading

thejcannon commented Nov 30, 2022

thejcannon commented Dec 1, 2022 •

edited

Loading

thejcannon Dec 7, 2022

thejcannon commented Dec 7, 2022 •

edited

Loading

jsirois commented Dec 7, 2022

thejcannon commented Dec 7, 2022

jsirois commented Dec 7, 2022

thejcannon commented Dec 7, 2022

Test DebugAdapter requests, and fix issues #17678

Test DebugAdapter requests, and fix issues #17678

Conversation

thejcannon commented Nov 29, 2022 • edited Loading

jsirois commented Nov 29, 2022

thejcannon commented Nov 29, 2022

jsirois commented Nov 29, 2022

thejcannon commented Nov 29, 2022

CJTurpie commented Nov 30, 2022

thejcannon commented Nov 30, 2022

thejcannon commented Nov 30, 2022

thejcannon commented Nov 30, 2022

thejcannon commented Nov 30, 2022

thejcannon commented Nov 30, 2022 • edited Loading

thejcannon commented Nov 30, 2022

thejcannon commented Dec 1, 2022 • edited Loading

thejcannon Dec 7, 2022

Choose a reason for hiding this comment

thejcannon commented Dec 7, 2022 • edited Loading

jsirois commented Dec 7, 2022

thejcannon commented Dec 7, 2022

jsirois commented Dec 7, 2022

thejcannon commented Dec 7, 2022

thejcannon commented Nov 29, 2022 •

edited

Loading

thejcannon commented Nov 30, 2022 •

edited

Loading

thejcannon commented Dec 1, 2022 •

edited

Loading

thejcannon commented Dec 7, 2022 •

edited

Loading