feat: implement SQL-only installation for PGMQ#41
Conversation
- Added `install.py` for downloading and executing SQL installation scripts. - Introduced `install_pgmq_from_sql` and related functions for installation. - Updated CI workflows to include SQL install tests. - Enhanced documentation for SQL installation and usage. - Added tests for SQL installation functionality.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces SQL-only installation utilities for PGMQ, allowing users to download and execute the PGMQ SQL schema directly from GitHub using Python (both synchronously and asynchronously). It includes updates to the configuration logic to conditionally skip extension initialization via the PG_INIT_EXTENSION environment variable, adds comprehensive unit and integration tests, and updates the documentation and Makefile. The code review feedback is highly constructive and should be addressed: it identifies a bug in _resolve_config where configuration arguments are ignored when a DSN is provided, suggests adding support for GitHub token authentication to avoid API rate limits, recommends handling potential JSON parsing errors when fetching release tags, and suggests adding a missing type annotation for consistency.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
- Removed tests related to GitHub-based installation and version fetching. - Introduced tests for embedded SQL version and installation. - Updated assertions to use EMBEDDED_SQL_VERSION instead of hardcoded version. - Simplified async installation test by removing unnecessary class structure. - Ensured proper cleanup of databases after tests.
tavallaie
changed the title
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces SQL-only installation utilities for PGMQ, allowing offline installation directly from Python using a bundled SQL script. It also adds support for disabling automatic extension initialization via the PG_INIT_EXTENSION environment variable, updates the testing environment to support plain Postgres instances, and adds corresponding integration tests. Feedback on the changes suggests filtering config_kwargs in _resolve_config to prevent TypeError from unexpected arguments, and wrapping the database connection attempt in install_pgmq_sql inside the try...except block to ensure connection errors are consistently wrapped in PGMQInstallError.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
…rove error handling in install_pgmq_sql
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces offline, SQL-only installation support for PGMQ directly from Python by bundling the pgmq.sql script (v1.11.1) and adding new installation utilities, documentation, and testing environments. The review feedback highlights three key improvement opportunities in src/pgmq/install.py: preventing DATABASE_URL from silently overriding explicit connection parameters in _resolve_config, using resolved_config.conn_string or resolved_config.dsn to preserve extra connection options, and wrapping the embedded SQL file read in a try-except block to ensure consistent exception handling with PGMQInstallError.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
…connection logic in install_pgmq_sql
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a Python-based SQL installation utility for PGMQ, allowing users to perform a fresh SQL-only install using a bundled SQL script. It updates the Makefile, documentation, and client configurations to support running and testing against a plain PostgreSQL instance with extension initialization disabled. Review feedback suggests addressing an issue in _resolve_config where standard psycopg connection parameters are silently discarded, and recommends using psycopg.sql.Identifier instead of f-string interpolation in tests to safely construct dynamic SQL statements.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Extract connection resolution from the install path into base.py so PGMQConfig construction is consistent across the library. - Reject unsupported libpq kwargs with a clear ValueError - Clear conn_string when explicit PGMQConfig fields are provided so DATABASE_URL does not override them - Use the helper from BaseQueue and reject mixing config with kwargs
- Use resolve_pgmq_config from base instead of a private duplicate - Parse bundled SQL version from the script header marker - Validate empty SQL input before connecting - Remove commented-out GitHub install code and stale exports - Document autocommit=False requirement for caller-owned connections
- Test resolve_pgmq_config from base instead of private install helper - Add empty SQL, SQL execution failure, and reinstall failure tests - Rename misleading async database fixture to second-database naming - Use psycopg.sql.Identifier for CREATE/DROP DATABASE in test helpers
- Run test-sql-install-env from make test after the main suite - Target plain Postgres via PG_SQL_INSTALL_* for install and tests - Replace full-suite test-sql-env in CI with focused install tests - Clear DATABASE_URL during make install-pgmq-sql to avoid surprises
- Add docs/sql_installation.md with API, env vars, and Makefile helpers - Link getting started to the new guide and remove stale HTML comments - Update AGENTS.md for install module, version 1.1.2, and test targets
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a bundled SQL installation feature for PGMQ, allowing users to install PGMQ objects directly into a PostgreSQL database when the native extension cannot be created. It adds the bundled SQL schema, Python installation utilities, updated configuration resolution, and integration tests against a plain Postgres instance. The review feedback highlights a critical regression in resolve_pgmq_config where passing non-connection keyword arguments (such as verbose or pool_size) incorrectly sets conn_string to None, silently disabling the use of DATABASE_URL from the environment. A code suggestion is provided to ensure conn_string is only overridden when explicit connection-specific fields are supplied.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
…e passed Only clear conn_string (disabling DATABASE_URL) when explicit connection fields such as host, port, database, username, or password are provided. Passing verbose, pool_size, or init_extension alone no longer falls back to localhost defaults.
ChuckHend
left a comment
ChuckHend
left a comment
There was a problem hiding this comment.
Thanks for taking this on! I do think it is going to cause problems if we duplicate the entire SQL codebase into src/pgmq/sql/pgmq.sql. It's almost certainly going to drift.
|
I already have functions to download it from GitHub (commented out). I'm concerned about GitHub rate limits and offline/air-gapped installations, so I included it in the library too. We can publish a new minor version for each pgmq.sql update. |
|
Could you pull it in during CI then publish it in the python package? |
|
I think it is possible, let me check that. |
|
@ChuckHend Which release should we consider? Check for each release of the |
Add a vendor script and workflow that opens a PR when the PGMQ extension repo dispatches pgmq-extension-release (or when run manually with a tag). Document the repository_dispatch hook upstream maintainers should add to pgmq/pgmq release workflows.
|
All the releases are semver tagged. IMO, we should pin to one of those versions. Run the client tests on that version and then package release it. |
There was a problem hiding this comment.
All the releases are semver tagged. IMO, we should pin to one of those versions. Run the client tests on that version and then package release it.
I added this action to check if pgmq has a new version, create a pull request. We have two options: trigger a dispatch action in pgmq, or run a cron job to check it.
There was a problem hiding this comment.
@ChuckHend, when a new file is added, like other PRs, the test will run on the new file.
There was a problem hiding this comment.
@ChuckHend, which of those two approaches should we choose? Trigger from the extension or a cron job?
There was a problem hiding this comment.
I think cron job from this project.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces support for a SQL-only installation of PGMQ directly from Python, enabling offline installations when the PostgreSQL extension cannot be created. It bundles the pgmq.sql script, adds installation utilities (install_pgmq_from_sql, install_pgmq_sql), implements configuration resolution helpers, updates documentation, and adds comprehensive integration and unit tests. Feedback on the changes suggests raising a ValueError if an existing connection conn is provided alongside other connection parameters to prevent silent configuration ignoring, and verifying that the provided connection does not have autocommit enabled to ensure the installation script runs within a transaction.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| own_conn = False | ||
| if conn is None: |
There was a problem hiding this comment.
When an existing connection conn is provided, any other connection parameters (config, dsn, or keyword arguments) are silently ignored. To prevent configuration errors and silent failures, we should raise a ValueError if conn is supplied alongside any other connection parameters. Additionally, we should verify that the provided connection does not have autocommit enabled, as the installation script must run within a transaction.
if conn is not None:
if config is not None or dsn is not None or config_kwargs:
raise ValueError("Cannot provide connection parameters (config, dsn, or kwargs) when an existing connection 'conn' is supplied.")
if getattr(conn, "autocommit", None) is True:
raise ValueError("The provided connection 'conn' must have autocommit=False to execute the installation script within a transaction.")
own_conn = False
if conn is None:Run vendor_pgmq_sql on a daily schedule that polls pgmq/pgmq releases, alongside the optional repository_dispatch fast path. Skip vendoring when the bundled version already matches so duplicate triggers are harmless.
|
@ChuckHend, I think we are ready for code review. |
ChuckHend
left a comment
ChuckHend
left a comment
There was a problem hiding this comment.
Nice work. I think we remove the pgmq.sql file and just rely on pulling that in during the build process, followed by running the client tests.
Also I think it is worth noting in documentation that the SQL only method does not support versioning, including upgrading versions. Those are two advantages of running this as a formal postgres extension over running it as "SQL only".
There was a problem hiding this comment.
I think cron job from this project.
| -- pgmq-py bundled SQL version: 1.11.1 | ||
| ------------------------------------------------------------ | ||
| -- Schema, tables, records, privileges, indexes, etc | ||
| ------------------------------------------------------------ | ||
| -- When installed as an extension, we don't need to create the `pgmq` schema | ||
| -- because it is automatically created by postgres due to being declared in | ||
| -- the extension control file | ||
| DO | ||
| $$ | ||
| BEGIN | ||
| IF (SELECT NOT EXISTS( SELECT 1 FROM pg_extension WHERE extname = 'pgmq')) THEN | ||
| CREATE SCHEMA IF NOT EXISTS pgmq; | ||
| END IF; | ||
| END | ||
| $$; | ||
|
|
There was a problem hiding this comment.
we should remove this file from the commit and just rely on copying it over during the build/release
There was a problem hiding this comment.
it is something related to core repo. or making a python script to remove these lines in this project.
There was a problem hiding this comment.
My concern is that there become two sources of truth for the server-side functionality. If we want to have ALL the pgmq extension code live together with pgmq-py, we might as well move pgmq-py back into the extension repo, then symlink the file.
I was thinking that the job would do this in CI, or something similar.
curl -O https://raw.githubusercontent.com/pgmq/pgmq/v1.1.1/pgmq-extension/sql/pgmq.sql
There was a problem hiding this comment.
let me work on it.
I think having pgmq.sql in the source code help us to trace or manually update if pulling from other repository failed. also I don't know if possible to have action to get the file and run test (I am not a very Github Action expert so I need to do lot of experiment) |
2 participants
install.pyfor downloading and executing SQL installation scripts.install_pgmq_from_sqland related functions for installation.