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

Fixes issue while trying to run / debug a pyramid application #530

Merged
merged 2 commits into from
Jan 4, 2018
Merged

Fixes issue while trying to run / debug a pyramid application #530

merged 2 commits into from
Jan 4, 2018

Conversation

jpfarias
Copy link

@jpfarias jpfarias commented Jan 4, 2018 •
edited by DonJayamanne
Loading

The debugger tries to find pserve on all platforms but on windows the correct file name should be pserve.exe.

Fixes #519

@jpfarias jpfarias changed the title Fixes issue #523 while trying to run / debug a pyramid application Fixes issue while trying to run / debug a pyramid application Jan 4, 2018
@jpfarias jpfarias mentioned this pull request Jan 4, 2018
Closed
@brettcannon
Copy link
Member

Thanks for the PR, @jpfarias ! Any chance you can look into why your PR isn't passing continuous integration?

@brettcannon
Copy link
Member

In another issue it was said the failure appeared to be network-related. I have restarted the jobs to see if that solves anything.

@jpfarias
Copy link
Author

jpfarias commented Jan 4, 2018

This is the list of errors, not sure if they are related with my PR though:

1) Standard Debugging Confirm debuggig works if both port and host are not provided:
     Uncaught Error: read ECONNRESET
      at exports._errnoException (util.js:1050:11)
      at Pipe.onread (net.js:581:26)
  2) Standard Debugging Confirm debuggig works if port=0:
     Uncaught Error: read ECONNRESET
      at exports._errnoException (util.js:1050:11)
      at Pipe.onread (net.js:581:26)
  3) Standard Debugging Confirm debuggig works if port=0 or host=localhost:
     Uncaught Error: read ECONNRESET
      at exports._errnoException (util.js:1050:11)
      at Pipe.onread (net.js:581:26)
  4) Standard Debugging Confirm debuggig works if port=0 or host=127.0.0.1:
     Uncaught Error: read ECONNRESET
      at exports._errnoException (util.js:1050:11)
      at Pipe.onread (net.js:581:26)
  5) Standard Debugging Confirm debuggig fails when an invalid host is provided:
     Uncaught Error: read ECONNRESET
      at exports._errnoException (util.js:1050:11)
      at Pipe.onread (net.js:581:26)
  6) Standard Debugging Confirm debuggig fails when provided port is in use:
     Uncaught Error: read ECONNRESET
      at exports._errnoException (util.js:1050:11)
      at Pipe.onread (net.js:581:26)

DonJayamanne
DonJayamanne requested changes Jan 4, 2018
View reviewed changes
src/client/debugger/Main.ts Outdated
@@ -22,6 +22,7 @@ import { DebugClient } from "./DebugClients/DebugClient";
import { CreateAttachDebugClient, CreateLaunchDebugClient } from "./DebugClients/DebugFactory";
import { BaseDebugServer } from "./DebugServers/BaseDebugServer";
import { PythonProcess } from "./PythonProcess";
import { IS_WINDOWS } from '../../../common/utils';
Copy link

Choose a reason for hiding this comment

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

Please do not use the common/utils found under client/common.
You'll need to use the IS_WINDOWS variable in Common/Utils`

This is because the global vscode object cannot exist (nor be imported) in the debugger process (it will break it, at least in the current version of VS Code).

src/client/debugger/Main.ts Outdated
@@ -214,11 +215,15 @@ export class PythonDebugger extends DebugSession {
catch (ex) {
}
if (Array.isArray(args.debugOptions) && args.debugOptions.indexOf("Pyramid") >= 0) {
let pserve = "pserve";
Copy link

Choose a reason for hiding this comment

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

please use an immediate and declare it as a constant

const pserve = ISWINDOWS ? 'pserve.exe' : 'pserve'

@DonJayamanne
Copy link

DonJayamanne commented Jan 4, 2018 •
edited
Loading

@jpfarias

This is the list of errors, not sure if they are related with my PR though:

The most likely cause of the failing tests is the issue I identified above.

updating code based on comments from DonJayamanne, changed the import…
09155e7 
… path for IS_WINDOWS and made pserve an immediate declared as const
@codecov
Copy link

codecov bot commented Jan 4, 2018 •
edited
Loading

Codecov Report

Merging #530 into master will increase coverage by 0.02%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #530      +/-   ##
==========================================
+ Coverage   56.42%   56.44%   +0.02%     
==========================================
  Files         212      212              
  Lines        9968     9970       +2     
  Branches     1755     1756       +1     
==========================================
+ Hits         5624     5628       +4     
+ Misses       4340     4338       -2     
  Partials        4        4
Impacted Files Coverage Δ
src/client/debugger/Main.ts 0% <0%> (ø) ⬆️
src/client/providers/lintProvider.ts 82.89% <0%> (+2.63%) ⬆️

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 4372809...09155e7. Read the comment docs.

@DonJayamanne
Copy link

@brettcannon
I'm merging this in-spite of the lack of tests (codecov/path = 0) as we don't have the ability to easily add tests for the debugger.

@DonJayamanne DonJayamanne merged commit 8f97908 into microsoft:master Jan 4, 2018
@jpfarias
Copy link
Author

jpfarias commented Jan 4, 2018

Thanks, I am glad I could help.

@JDeuce JDeuce mentioned this pull request Aug 22, 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
No milestone
Development

Successfully merging this pull request may close these issues.

launch.json Pyramid debug option tries to launch the wrong program on Windows
3 participants
@jpfarias @brettcannon @DonJayamanne