Skip to content

Enable continuous debugging for Python functions #2576

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

Merged
merged 4 commits into from
Dec 16, 2020
Merged

Enable continuous debugging for Python functions #2576

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8eb7b0b
No need to wait for debugger attach
Dec 7, 2020
bbbfb64
No need to disable subprocess
Dec 11, 2020
f8d23eb
Add a feature flag to control --wait-for-client
Dec 15, 2020
6d739ad
Revert "Add a feature flag to control --wait-for-client"
Dec 16, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/debug/PythonDebugProvider.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ async function getPythonCommand(host: string, port: number): Promise<string> {

Copy link
Contributor

Choose a reason for hiding this comment

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

Related to discussion in Azure/azure-functions-python-worker#607 with @mhoeger, one of the scenarios mentioned by users is as follows:

  1. Create a simple python project with an http trigger
  2. Make sure "AzureWebJobsStorage" is set to "UseDevelopmentStorage=true"
  3. Make sure the local emulator is not running and start debugging

With your change, it works. Without your change, I get "The operation has timed out". This scenario isn't related to "continuous debugging" - so can you explain why your change helps?

Copy link
Contributor Author

@Hazhzeng Hazhzeng Dec 15, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Hi @ejizba, this is because in the vscode, when hitting F5, it does not start the Python azure_functions_worker directly, but starts the debugpy Python module provided by ms.python extension.

The debugpy is used for hosting a Python debug server for the Python code (debugee). This debugpy accepts a --wait-for-client flag. When this flag is set to true, it will not start the Python azure_functions_worker until the vscode debugger connects to it.

From my understanding, the vscode-azurefunctions extension has a problem matcher Worker process started and initialized|Host lock lease acquired by instance ID, which only connects the vscode debugger to debugee after the Python worker starts. This creates a racing condition.

Let me know if you have any questions.

Copy link
Contributor

Choose a reason for hiding this comment

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

Right so this is my understanding of the current ordering with "wait-for-client" flag:

  1. Start func cli
  2. Func cli starts debugpy
  3. debugpy waits for client to attach once "Worker process is started and initialized" is printed by func cli
  4. debugpy starts the worker

So my question is - how does "Worker process is started and initialized" ever get printed if debugpy waits until step 4 to start the worker? And why does the local emulator affect that?

Hmm now that I think about it - is it because "host lock lease" is what's triggering step 3 (since the problem matcher has two options) and that won't be printed if the emulator isn't running?

// tslint:disable-next-line:strict-boolean-expressions
if (pyExtension.exports && pyExtension.exports.debug) {
return (await pyExtension.exports.debug.getRemoteLauncherCommand(host, port)).join(' ');
return (await pyExtension.exports.debug.getRemoteLauncherCommand(host, port, false)).join(' ');
} else {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm about ready to approve, but one more question for ya. I can only assume the Python extension defaults this to "true" for a reason, so what are the downsides or risks of setting it to "false" and why don't those apply to us?

Copy link
Contributor Author

@Hazhzeng Hazhzeng Dec 15, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

If a customer breakpoints the import statement from very top of their code, the debugger may not be able to stop there since the debugger may attach after a while. This is just my theory and I haven't run into this issue yet.

To prevent any unforeseen issue, I introduce a feature flag azureFunctions.waitForPythonDebugger. This is set to false by default, but the customer can always set it to true to revert the new behavior.

Copy link
Contributor

Choose a reason for hiding this comment

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

It would more likely be a problem if they set a breakpoint at the top of the worker code, right? Not their code? And I don't see any reason they would try to debug the worker code. The only reason debugging will work with the "--wait-for-client" flag is because of the "host lock lease" message and I want us to move away from that in favor of "worker started and initialized". I appreciate the attempt to be more careful and add the setting, but that's not really what I was going for. Sounds like the worst case is the debugger will still attach and the vast majority of breakpoints will still work, except for maybe one at the beginning. With that in mind, I'd rather not add a setting that relies on the "host lock lease" message

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, sorry for the confusion. If the customer debug their code, they will not run into this issue.

throw new Error(localize('pyExtOutOfDate', 'You must update extension with id "{0}" to debug Python projects.', pyExtensionId));
}
Expand Down