Add explicit CLI handling for ptvsd.#266
Conversation
ptvsd/wrapper.py
Outdated
| # These are the functions pydevd invokes to get a socket to the client. | ||
| pydevd_comm.start_server = start_server | ||
| pydevd_comm.start_client = start_client | ||
| def install(start_server=start_server, start_client=start_client): |
There was a problem hiding this comment.
This neuters the assert that we have in pydevd/__init__.py about how pydevd must not be imported before ptvsd - now it can be imported before, but if you don't call install, the detours won't work.
There was a problem hiding this comment.
Unlike the first time I tried this, this function now forces a re-import of the "pydevd" module. Alternately, we could manually replace pydevd.start_server and pydevd.start_client.
Also, where is pydevd/__init__.py?
There was a problem hiding this comment.
Sorry, that was meant to be ptvsd/__init__.py.
What happens if some ptvsd code accidentally imports pydevd before we so the refresh below (e.g. to use some constant from it). Do we end up with two copies of that module hanging around in memory, but no harmful effects otherwise?
There was a problem hiding this comment.
I've updated the code to monkeypatch the start functions rather than re-importing the pydevd module. That fixes issues where pydevd was already imported. I've also added some comments on why we have to jump through these hoops.
Codecov Report
@@ Coverage Diff @@
## master #266 +/- ##
=========================================
- Coverage 59.94% 57.4% -2.54%
=========================================
Files 10 10
Lines 1735 1850 +115
=========================================
+ Hits 1040 1062 +22
- Misses 695 788 +93
Continue to review full report at Codecov.
|
|
Ohh, now I see what's really going on there. Thanks for figuring this out! |
3 participants
This change is intended to accommodate future work (adding more CLI options where using the launch config is insufficient). It also allows us to define a consistent and easy-to-discover CLI, independent of PyDevd.
To be backward compatible, the code also supports the PyDevd CLI (for now).