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.

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

Update async samples to use asyncio.run instead of loop #21207

Closed
yunhaoling opened this issue Oct 12, 2021 · 2 comments · Fixed by #26098
Closed

Update async samples to use asyncio.run instead of loop #21207

yunhaoling opened this issue Oct 12, 2021 · 2 comments · Fixed by #26098
Labels
Client This issue points to a problem in the data-plane of the library. Epic MQ This issue is part of a "milestone of quality" initiative.
Milestone

Comments

@yunhaoling
Copy link
Contributor

yunhaoling commented Oct 12, 2021

  • since Python 3.10 asyncio.get_event_loop is deprecated, a deprecation warning will be emitted if there is no running event loop. (For more context on the deprecation, please check doc: https://docs.python.org/3/library/asyncio-eventloop.html#event-loop)
  • In our repo, asyncio.get_event_loop is used broadly in async samples for execution which means our users will get warning when they execute async samples. example in EH:
    image
  • we should try our best to update our async samples to use asyncio.run instead of loop as suggested by the Python community:

Application developers should typically use the high-level asyncio functions, such as asyncio.run(), and should rarely need to reference the loop object or call its methods. This section is intended mostly for authors of lower-level code, libraries, and frameworks, who need finer control over the event loop behavior.

cc: @lmazuel

@yunhaoling yunhaoling added Client This issue points to a problem in the data-plane of the library. Epic MQ This issue is part of a "milestone of quality" initiative. labels Oct 12, 2021
@yunhaoling yunhaoling added this to the [2021] December milestone Oct 12, 2021
@Codejune
Copy link
Contributor

Codejune commented Oct 16, 2021

I have a question about this issue.

As asyncio.get_event_loop is deprecated, we can suggest several methods.

The first method, this is the lower-level method.

loop = asyncio.new_event_loop()
asyncio.set_event_loop(loop)
loop.run_until_complete(...)

The second method, this method was added in Python 3.7. The reference document officially recommends the following method.

loop = asyncio.get_running_loop()
loop.run_until_complete(...)

Last method, this method was also added in Python 3.7.

asyncio.run(...)

In the sample, the asyncio.run() method is not considered for using Python 3.6 or lower users.
So I'm a little concerned about this method.

@lmazuel
Copy link
Member

lmazuel commented Oct 18, 2021

Hi @Codejune ,

First on the Python versions: the baseline we support today is 3.6, that is going EOL this December 2021. We are not sure yet if we will deprecate 3.6 support immediately at its EOL, but soon in 2022 we will stop supporting 3.6 completely anyway. Which will make 3.7 the new baseline. There are no "lower users", since we stopped supporting 3.5 and below for a few months already.

The documented way in the Python doc today is asyncio.run, since 3.7:
https://docs.python.org/3.7/library/asyncio.html

This function always creates a new event loop and closes it at the end. It should be used as a main entry point for asyncio programs, and should ideally only be called once.

image

Following the recommendation of Python documentation do not prevent you to use loop.run_until_complete(...) :). It just means we believe we should follow the latest guidance, and sync with the official doc in our samples.

@lmazuel lmazuel modified the milestones: [2021] December, [2022] April Feb 18, 2022
@lmazuel lmazuel modified the milestones: [2022] April, [2022] November May 16, 2022
ghost pushed a commit that referenced this issue Sep 12, 2022
# Description

As #21207 mentions, some samples are using a loop variable for simple callbacks when this can be done by using the new run method of asyncio. This PR remedies the issue for the monitor library.

# All SDK Contribution checklist:
- [x] **The pull request does not introduce [breaking changes]**
- [x] **CHANGELOG is updated for new features, bug fixes or other significant changes.**
- [x] **I have read the [contribution guidelines](https://github.com/Azure/azure-sdk-for-python/blob/main/CONTRIBUTING.md).**

## General Guidelines and Best Practices
- [x] Title of the pull request is clear and informative.
- [x] There are a small number of commits, each of which have an informative message. This means that previously merged commits do not appear in the history of the PR. For more information on cleaning up the commits in your PR, [see this page](https://github.com/Azure/azure-powershell/blob/master/documentation/development-docs/cleaning-up-commits.md).

### [Testing Guidelines](https://github.com/Azure/azure-sdk-for-python/blob/main/CONTRIBUTING.md##building-and-testing)
- [x] Pull request includes test coverage for the included changes.
mccoyp pushed a commit to mccoyp/azure-sdk-for-python that referenced this issue Sep 22, 2022
# Description

As Azure#21207 mentions, some samples are using a loop variable for simple callbacks when this can be done by using the new run method of asyncio. This PR remedies the issue for the monitor library.

# All SDK Contribution checklist:
- [x] **The pull request does not introduce [breaking changes]**
- [x] **CHANGELOG is updated for new features, bug fixes or other significant changes.**
- [x] **I have read the [contribution guidelines](https://github.com/Azure/azure-sdk-for-python/blob/main/CONTRIBUTING.md).**

## General Guidelines and Best Practices
- [x] Title of the pull request is clear and informative.
- [x] There are a small number of commits, each of which have an informative message. This means that previously merged commits do not appear in the history of the PR. For more information on cleaning up the commits in your PR, [see this page](https://github.com/Azure/azure-powershell/blob/master/documentation/development-docs/cleaning-up-commits.md).

### [Testing Guidelines](https://github.com/Azure/azure-sdk-for-python/blob/main/CONTRIBUTING.md##building-and-testing)
- [x] Pull request includes test coverage for the included changes.
@github-actions github-actions bot locked and limited conversation to collaborators Apr 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Client This issue points to a problem in the data-plane of the library. Epic MQ This issue is part of a "milestone of quality" initiative.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants