Fix wrong event loop usage from PGMQueue

[jason810496]

No description provided.

[gemini-code-assist]

Summary of Changes

Hello @jason810496, I'm Gemini Code Assist¹! 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 focuses on refining the PGMQueue class by removing its internal asyncio event loop management. This change simplifies the class's responsibilities, promoting a more externalized control over event loops. The accompanying test suite has been updated to reflect this architectural shift, including the removal of obsolete event loop tests and the introduction of smarter test parametrization to ensure appropriate execution based on the chosen database driver.

Highlights

• Decoupled Event Loop Management: The PGMQueue class no longer directly manages asyncio event loops, removing the 'loop' attribute and constructor parameter. This simplifies the class and delegates event loop responsibility to the caller.
• Streamlined PGMQ Extension Checks: Internal asynchronous and synchronous methods for checking the PGMQ extension (_check_pgmq_ext_async, _check_pgmq_ext_sync) have been removed, consolidating the logic within a single _check_pgmq_ext method.
• Removed Event Loop Specific Tests: The 'test_event_loop.py' file, which contained tests for event loop handling, has been deleted as its functionality is no longer relevant to the simplified PGMQueue design.
• Enhanced Test Parametrization: The test suite now includes logic to automatically skip synchronous tests when an asynchronous database driver is specified, improving test efficiency and relevance.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

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 by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

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 pull request 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. ↩

[gemini-code-assist]

Code Review

This pull request aims to refactor the event loop handling in PGMQueue, which is a commendable goal. However, the current implementation is incomplete and introduces a critical bug. While the loop attribute and its initialization are removed, several methods still attempt to call self.loop.run_until_complete(), which will cause an AttributeError at runtime. This breaks all asynchronous operations when they are invoked from synchronous methods. I have left a detailed comment explaining the issue and suggesting potential fixes. The other changes, such as adding type hints and improving formatting in the test suite, are positive improvements.

I am having trouble creating individual review comments. Click here to see my feedback.

pgmq_sqlalchemy/queue.py (110-117)

This change removes the loop attribute and its initialization. However, other parts of the class, specifically the _execute_operation method, still rely on self.loop to run asynchronous operations when is_async is true. This will lead to an AttributeError at runtime.

The PR's goal to fix event loop usage is good, but the implementation is incomplete. You need to decide how to handle the synchronous invocation of asynchronous operations.

Here are two possible solutions:

1. Use asyncio.run() (with caution):
   You could re-import asyncio and replace self.loop.run_until_complete(...) with asyncio.run(...). Be aware that asyncio.run() cannot be called when an event loop is already running, which might be problematic for users who are already in an async context.

2. Separate sync and async APIs (Recommended):
   A cleaner approach is to provide distinct async methods and require users to await them when using an async engine. The synchronous methods would then raise an error if called on an async queue. Given that you've removed the test test_execute_operation_async_with_session_none, it seems you might be leaning towards this option, which is generally better for library design. If so, the _execute_operation method and all public-facing methods need to be refactored to enforce this separation.

pgmq_sqlalchemy/queue.py (121-130)

These methods (_check_pgmq_ext_async and _check_pgmq_ext_sync) have been removed, and the call to their wrapper _check_pgmq_ext() has also been removed from __init__. However, the _check_pgmq_ext method itself has been left behind. Based on the full file content, it now calls these removed methods, making it broken. Since it's no longer called, it appears to be dead code. It would be cleaner to remove the _check_pgmq_ext method as well to avoid confusion and prevent future use of broken code.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

Files with missing lines	Coverage Δ
pgmq_sqlalchemy/queue.py	87.50% <ø> (-10.80%)	⬇️

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.