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

Deprecation warning in examples #246

Closed
terop opened this issue Mar 11, 2024 · 3 comments · Fixed by #257
Closed

Deprecation warning in examples #246

terop opened this issue Mar 11, 2024 · 3 comments · Fixed by #257
Labels

Comments

@terop
Copy link
Contributor

terop commented Mar 11, 2024

Examples (and some code too) use the asyncio.get_event_loop().run_until_complete() function call chain to run asynchronous functions.
With Python 3.12 a DeprecationWarning is emitted: ruuvitag-sensor/examples/get_async_bleak.py:18: DeprecationWarning: There is no current event loop. Python get_event_loop() documentation recommends to either use get_running_loop() or asyncio.run(). Using get_running_loop() results in the below error so it is not a straightforward replacement.

$ python3 examples/get_async_bleak.py
Traceback (most recent call last):
  File "<redacted>/ruuvitag-sensor/examples/get_async_bleak.py", line 18, in <module>
    asyncio.get_running_loop().run_until_complete(main())
    ^^^^^^^^^^^^^^^^^^^^^^^^^^
RuntimeError: no running event loop

I am wondering what would be the best way to fix the DeprecationWarning.
There is no rush to fix this but fixing this before the call results in an error would be advisable.

Environment:

  • OS: Gentoo
  • ruuvitag_sensor package version: master (2.3.1)
  • RuuviTag firmware version: not relevant
  • Data format: not relevant
@terop terop added the bug label Mar 11, 2024
@terop
Copy link
Contributor Author

terop commented Oct 14, 2024

@ttu could you kindly provide some input on this?

@ttu
Copy link
Owner

ttu commented Oct 15, 2024

Like you mentioned we should replace asyncio's get_event_loop().run_until_complete() with run().

In RuuviTag's codebase, asyncio is not used in many places (main, Bleak, RX, examples, and documentation), but we should still ensure that everything works correctly after the change.

At least these come to my mind now. Can you think something I might have missed?

  • Bleak works normally after the change to asyncio.run().
  • RuuviTagReactice uses loop = asyncio.get_event_loop(). Should we use get_running_loop here? code example
  • DevBleakScanner uses asyncio.create_task(self.run()). Does this work or does it require some change? code
  • There was the issue with asyncio.run() and Bleak adapter with Python 3.9. Can we find a solution for this or in worse case drop support for 3.9? BleCommunicationBleak doesn't work with asyncio.run() on Python 3.9 #186 (comment)

@terop
Copy link
Contributor Author

terop commented Nov 11, 2024

Sorry for the late reply. Please find some answers to your questions below. I will open a PR to update the examples shortly.

Bleak works normally after the change to asyncio.run().

This seems to be the case per my testing.

RuuviTagReactice uses loop = asyncio.get_event_loop(). Should we use get_running_loop here?

I didn't see any error when trying to run an example using RuuviTagReactive still having asyncio.get_event_loop(). Per the documentation using get_event_loop() is still okay when an event loop is running and this seems to be the case with RuuviTagReactive.

DevBleakScanner uses asyncio.create_task(self.run()). Does this work or does it require some change?

Per Python documentation using asyncio.create_task() should still be okay.

There was the issue with asyncio.run() and Bleak adapter with Python 3.9. Can we find a solution for this or in worse case drop support for 3.9?

This is a tricky question. The fix proposed by you in the issue indeed works but should it and how should it be applied to the code? Some kind of Python "#ifdef" would be possible but it seems quite dirty to me. About dropping Python 3.9 support: are there any statistics which would show many users still using Python 3.9 downloading this package?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants