Skip to content

Conversation

Hazhzeng
Copy link
Contributor

@Hazhzeng Hazhzeng commented Dec 8, 2020

Thanks @mhoeger for bringing this up and suggesting for the --wait-for-client change. As Azure Functions Python also want to enable hot-reload debugging on Azure Functions.

Background

Testing with "azureFunctions.stopFuncTaskPostDebug": false in .vscode/settings.json, everytime when a user is in a debug session, making changes to the function code will result in a timeout issue. This is mainly caused by:

  1. When the host restart the Python worker, it actually restart the debugpy program instead, e.g.
Starting worker process with FileName:py WorkingDirectory:C:\Users\hazeng\Projects\Functions\PythonVSCodeDebug 
Arguments: c:/Users/hazeng/.vscode/extensions/ms-python.python-2020.11.371526539/pythonFiles/lib/python/debugpy
 --listen 127.0.0.1:9091
 --wait-for-client
 "C:\Users\hazeng\Projects\Artifacts\_\workers\python\3.7/WINDOWS/X64/worker.py"
 --host 127.0.0.1
 --port 23161
 --workerId 7ce43683-d867-48ad-9546-d366e0ba1aa1
 --requestId 107191ad-ab4e-45e6-9383-42c430796b04
 --grpcMaxMessageLength 2147483647
  1. As the --wait-for-client flag generated from PythonDebugProvider.ts which will block the Python worker from starting (as the vscode debugger quits on file change). In this case, we want to remove the --wait-for-client flag, and let the host restarts the worker.

Testing

  1. Download this branch and hit F5 to enable debug
  2. Create a new python function app project
  3. Add a configuration in .vscode/settings.json, azureFunctions.stopFuncTaskPostDebug: false
  4. Hit F5 to start debug the Python worker, the host should now spin up a debugpy server with Azure Functions Python Worker running inside.
  5. Change some code in the middle of debugging section, and the host should restart the debugpy server with the latest user code change.
  6. Reattach the debugger by pressing F5 again. The host never exits as the debugger should now be attached in the same host process.

cc: @anirudhgarg @vrdmr @mhoeger @ejizba

@Hazhzeng Hazhzeng requested a review from a team as a code owner December 8, 2020 00:02
@Hazhzeng
Copy link
Contributor Author

Hazhzeng commented Dec 9, 2020

Seems like this DurableFunctions CI error is impacting the main branch as well.

if (!pyExtension.isActive) {
await pyExtension.activate();
}

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

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?

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

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.

@ejizba ejizba merged commit 9b7bc03 into microsoft:main Dec 16, 2020
@dirkesquire
Copy link

The change was merged in, but 4 tests failed. Now what?

@github-actions github-actions bot locked and limited conversation to collaborators Jan 31, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants