Skip to content

Add Python debugging support#2205

Merged
balopat merged 20 commits intoGoogleContainerTools:masterfrom
briandealwis:dbg-python
Jun 19, 2019
Merged

Add Python debugging support#2205
balopat merged 20 commits intoGoogleContainerTools:masterfrom
briandealwis:dbg-python

Conversation

@briandealwis
Copy link
Copy Markdown
Member

@briandealwis briandealwis commented May 30, 2019

This patch adds support to skaffold debug for Python 2.7 and 3.x debugging. Python requires additional debugging runtime support files, which are fetched and installed using an initContainer base.

These debugging runtime support files are installed via initContainers from images defined in https://github.com/GoogleContainerTools/container-debug-support/. These images are currently manually pushed to gcr.io/gcp-dev-tools/duct-tape/XXX where XXX is a runtime-specific identifier like python (supports both Python 2.7 and 3.7).

The Python image installs the ptvsd module, a wrapper that uses the IDE/editor-agnostic debug adapter protocol (DAP). This protocol has become a de facto standard for interacting with remote runtimes.

Manual Testing

For manual testing, you'll need to use VS Code or Eclipse with LSP4e, or some other editor that supports the DAP. I had to launch my debug session with the following launch parameters, to map the local file system to the remote file system in the container:

{"pathMappings": [ { "localRoot": "_/your/local/path_", "remoteRoot": "/app" }]}

The debugger is on port 5678.

Fixes #2173

@codecov-io
Copy link
Copy Markdown

codecov-io commented May 31, 2019

Codecov Report

Merging #2205 into master will increase coverage by 0.45%.
The diff coverage is 87.12%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2205      +/-   ##
==========================================
+ Coverage    58.6%   59.06%   +0.45%     
==========================================
  Files         188      189       +1     
  Lines        7805     7924     +119     
==========================================
+ Hits         4574     4680     +106     
- Misses       2859     2866       +7     
- Partials      372      378       +6
Impacted Files Coverage Δ
pkg/skaffold/util/util.go 76.59% <100%> (+1.04%) ⬆️
pkg/skaffold/debug/transform_jvm.go 94.89% <100%> (+0.1%) ⬆️
pkg/skaffold/debug/debug.go 44.82% <50%> (ø) ⬆️
pkg/skaffold/debug/transform_nodejs.go 79.24% <66.66%> (+0.39%) ⬆️
pkg/skaffold/debug/transform_python.go 85.55% <85.55%> (ø)
pkg/skaffold/debug/transform.go 85.03% <96%> (+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 342b210...5d27b3d. Read the comment docs.

}
if len(supportFilesRequired) > 0 {
logrus.Infof("Configuring installation of debugging support files")
supportVolume := v1.Volume{Name: "debugging-support-files", VolumeSource: v1.VolumeSource{EmptyDir: &v1.EmptyDirVolumeSource{}}}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

"debugging-support-files" is used on multiple lines. Extract to constant?

Copy link
Copy Markdown
Member

@loosebazooka loosebazooka left a comment

Choose a reason for hiding this comment

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

Just some stuff to clarify things for me?

@briandealwis
Copy link
Copy Markdown
Member Author

PTAL @loosebazooka @ILMTitan

@briandealwis
Copy link
Copy Markdown
Member Author

@quoctruong PTAL

@briandealwis briandealwis added the kokoro:run runs the kokoro jobs on a PR label Jun 17, 2019
@kokoro-team kokoro-team removed the kokoro:run runs the kokoro jobs on a PR label Jun 17, 2019
Copy link
Copy Markdown
Member

@loosebazooka loosebazooka left a comment

Choose a reason for hiding this comment

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

Just some language stuff I'm not sure about, feel free to ignore or whatever.

@loosebazooka
Copy link
Copy Markdown
Member

Also might be worth doing a ux study with the team to see if we can all follow the instructions 🛩️

@briandealwis
Copy link
Copy Markdown
Member Author

PTAL @dgageot @balopat — need your approvals due to doc changes

Copy link
Copy Markdown
Contributor

@balopat balopat left a comment

Choose a reason for hiding this comment

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

LGTM.

Re Approvals: you had changes in a main util file hence our review was required. Also more "debug" related files were changed for integration testing that weren't owned by you, so I added those to the CODEOWNERS in #2292

@balopat balopat merged commit e59fc2f into GoogleContainerTools:master Jun 19, 2019
@briandealwis briandealwis deleted the dbg-python branch November 11, 2019 20:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add support for Python for skaffold debug

8 participants