Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add the geventCompatible launch configuration option #1699

Merged
merged 3 commits into from
Jun 7, 2018
Merged

Add the geventCompatible launch configuration option #1699

merged 3 commits into from
Jun 7, 2018

Conversation

underyx
Copy link

@underyx underyx commented May 16, 2018

Fixes #127

This pull request:

  • Has a title summarizes what is changing
  • Includes a news entry file (remember to thank yourself!)
  • Has unit tests & code coverage is not adversely affected (within reason) — agreed with @DonJayamanne to not add a test for this
  • Works on all actively maintained versions of Python (e.g. Python 2.7 & the latest Python 3 release) — untested
  • Works on Windows 10, macOS, and Linux (e.g. considered file system case-sensitivity) — untested, but there's no file system operations touched

@msftclas
Copy link

msftclas commented May 16, 2018 •
edited
Loading

CLA assistant check
All CLA requirements met.

@underyx underyx mentioned this pull request May 16, 2018
Closed
DonJayamanne
DonJayamanne requested changes May 16, 2018
View reviewed changes
Copy link

@DonJayamanne DonJayamanne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • We're missing code to create environment variable.
  • @brettcannon please review messages.

news/1 Enhancements/127.md Outdated
@@ -0,0 +1,2 @@
Add the `geventCompatible` launch configuration option,
to make the experimental Python debugger work with projects using gevent's monkey patching.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd change this to:
Add geventCompatible launch configuration option to enable debugging of gevent monkey patched code.

src/client/debugger/Common/Contracts.ts Outdated
@@ -54,6 +54,7 @@ export type DebuggerType = 'python' | 'pythonExperimental';
export interface AdditionalLaunchDebugOptions {
redirectOutput?: boolean;
django?: boolean;
geventCompatible?: boolean;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rename property to gevent

package.nls.json Outdated
@@ -39,6 +39,8 @@
"python.snippet.launch.flask.description": "Debug a Flask Application",
"python.snippet.launch.flaskOld.label": "Python: Flask (0.10.x or earlier)",
"python.snippet.launch.flaskOld.description": "Debug an older styled Flask Application",
"python.snippet.launch.gevent.label": "Python: Gevent-compatible",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove the word -compatible

package.json Outdated
@@ -975,6 +986,11 @@
"description": "Debug standard library code.",
"default": false
},
"geventCompatible": {
"type": "boolean",
"description": "Make the debugger compatible with gevent monkey-patching.",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please change to Enable debugging of gevent monkey-patched code

@codecov
Copy link

codecov bot commented May 16, 2018 •
edited
Loading

Codecov Report

Merging #1699 into master will decrease coverage by 2.96%.
The diff coverage is 80%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1699      +/-   ##
==========================================
- Coverage   74.44%   71.48%   -2.97%     
==========================================
  Files         282      277       -5     
  Lines       13253    13016     -237     
  Branches     2382     2345      -37     
==========================================
- Hits         9866     9304     -562     
- Misses       3257     3576     +319     
- Partials      130      136       +6
Impacted Files Coverage Δ
src/client/debugger/Common/Contracts.ts 100% <ø> (ø) ⬆️
src/client/common/variables/environment.ts 92.45% <100%> (-4.92%) ⬇️
src/client/debugger/DebugClients/helper.ts 92.85% <66.66%> (-3.3%) ⬇️
.../unittests/common/services/configSettingService.ts 6.81% <0%> (-85.02%) ⬇️
src/client/unittests/display/main.ts 5.88% <0%> (-81.62%) ⬇️
src/client/unittests/configuration.ts 19.51% <0%> (-76.79%) ⬇️
...preter/locators/services/windowsRegistryService.ts 19.48% <0%> (-74.11%) ⬇️
src/client/providers/symbolProvider.ts 23.33% <0%> (-59.36%) ⬇️
...ttests/common/managers/testConfigurationManager.ts 10.63% <0%> (-25.73%) ⬇️
src/client/common/installer/productInstaller.ts 59.18% <0%> (-18.5%) ⬇️
... and 55 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7f48a19...3968ec1. Read the comment docs.

@underyx
Add the geventCompatible launch configuration option
54f8b7f 
This makes the experimental Python debugger work with projects using
gevent's monkey patching.

Closes #127
@underyx
Copy link
Author

underyx commented May 16, 2018 •
edited
Loading

  • Inline comments all resolved

  • We're missing code to create environment variable

    Yup, my bad, messed up the commit split as I was fixing tslint errors; now fixed

Thanks for the comments!

@underyx
Copy link
Author

underyx commented May 25, 2018

@brettcannon FYI I think this is waiting for you only.

@brettcannon
Copy link
Member

Labels seem fine to me. Now we're just waiting on @DonJayamanne to give the all-clear and to merge!

@DonJayamanne DonJayamanne added this to the June 2018 milestone Jun 5, 2018
@DonJayamanne DonJayamanne merged commit b1055e1 into microsoft:master Jun 7, 2018
@brettcannon brettcannon mentioned this pull request Jun 7, 2018
Closed
@lock lock bot locked as resolved and limited conversation to collaborators Jul 31, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@DonJayamanne DonJayamanne DonJayamanne approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
June 2018
Development

Successfully merging this pull request may close these issues.

Make debugger gevent compatible
4 participants
@underyx @msftclas @brettcannon @DonJayamanne