Skip to content

Conversation

@pt2302
Copy link
Contributor

@pt2302 pt2302 commented Aug 28, 2025

What are the relevant tickets?

Closes https://github.com/mitodl/hq/issues/8196.

Description (What does it do?)

This PR refactors the use of asyncio event loops to correspond to the modern practice of using asyncio.run instead of manually getting the current event loop, which is deprecated and was causing tests to fail. It also pins the requirements to the latest versions, so that the test environment for future PRs is predictable. Future version updates can be handled by renovate[bot].

See https://docs.python.org/3/library/asyncio-eventloop.html#asyncio.get_event_loop for related documentation on the changes to asyncio.

How can this be tested?

All of the tests should now pass.

@pt2302 pt2302 requested a review from Copilot August 28, 2025 22:10
Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Summary of Changes

Hello @pt2302, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request modernizes the application's asynchronous operations by migrating from deprecated asyncio event loop management to the contemporary asyncio.run function and asyncio.create_task. This change addresses previous test failures and aligns the codebase with current Python best practices. Additionally, it pins all project dependencies to specific versions, ensuring a predictable and stable test environment for future development.

Highlights

  • Modernized Asyncio Usage: The application's asynchronous operations have been modernized by migrating from deprecated manual event loop management to the contemporary asyncio.run function for main entry points and asyncio.create_task for task creation.
  • Removed Deprecated Event Loop Parameter: The loop parameter, previously used for passing the asyncio event loop, has been removed from the Bot and DoofSpoof constructors and their instantiations, streamlining the API and removing a deprecated pattern.
  • Pinned Dependencies: All direct and test dependencies are now explicitly version-locked in requirements.txt and test_requirements.txt, ensuring a predictable and consistent environment for development and testing.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in issue comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR modernizes asyncio usage by replacing deprecated asyncio.get_event_loop() calls with asyncio.run() and asyncio.create_task(), while also pinning test dependencies to specific versions for predictable testing environments.

  • Replaces deprecated asyncio.get_event_loop() with modern asyncio.run() pattern
  • Updates task creation to use asyncio.create_task() instead of loop.create_task()
  • Pins test dependencies to specific versions for reproducible builds

Reviewed Changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
bot.py Removes loop parameter from Bot constructor and updates task creation calls
bot_local.py Replaces manual event loop handling with asyncio.run()
bot_test.py Updates DoofSpoof test class to remove loop dependency
web.py Adds asyncio import and updates task creation to use asyncio.create_task()
web_test.py Removes asyncio import and loop setup from test class
test_requirements.txt Pins all test dependencies to specific versions

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

bot.py Outdated
loop = asyncio.get_event_loop()
loop.run_until_complete(async_main())
loop.run_forever()
asyncio.run(async_main())
Copy link

Copilot AI Aug 28, 2025

Choose a reason for hiding this comment

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

The async_main() function contains an infinite event loop (app.listen() and the subsequent startup tasks), but asyncio.run() will complete when async_main() returns. This will cause the bot to exit immediately instead of running continuously. The original pattern with loop.run_forever() was correct for a long-running service.

Suggested change
asyncio.run(async_main())
loop = asyncio.get_event_loop()
loop.create_task(async_main())
loop.run_forever()

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

That would put back the old pattern. Updated in e128805 to use Tornado to keep the loop running explicitly.

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request does a great job of modernizing the asyncio usage by replacing the deprecated get_event_loop with asyncio.run and asyncio.create_task. This significantly improves the code's clarity and adherence to current best practices. Pinning dependencies is also an excellent step towards ensuring reproducible builds. I've found one critical issue with an invalid package version that needs to be addressed.

pdbpp==0.11.7
pylint==3.3.8
pytest==8.4.1
pytest-asyncio==1.1.0

Choose a reason for hiding this comment

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

critical

The version 1.1.0 for pytest-asyncio does not exist on PyPI. This will cause dependency installation to fail. It seems to be a typo. Please correct it to a valid version. The latest version is 0.23.7, which you could use if it's compatible with your setup.

pytest-asyncio==0.23.7

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

@rhysyngsun rhysyngsun left a comment

Choose a reason for hiding this comment

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

LGTM

@pt2302 pt2302 merged commit cf90f1c into master Aug 29, 2025
5 checks passed
@pt2302 pt2302 deleted the pt/refactor_asyncio branch August 29, 2025 16:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants