Add skip_permissions_check option to Psycopack
#45
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Previously, the permissions check was done each time a Psycopack object was instantiated. The check looks for the permissions needed for the complete operation, including all the individual steps. However, one step, backfill(), doesn't require the much higher elevated permissions that the other steps require. Following the principle of least privilege, it might be desirable to run the backfill() step using a Postgres user with lower privileges than the user used for the other steps. This change adds a skip_permissions_check option to Psycopack which enables its use with a less-privileged Postgres user.
Coverage Report Results
1 empty file skipped. |
There was a problem hiding this comment.
Pull Request Overview
This PR adds a new option, skip_permissions_check, to the Psycopack class to allow bypassing permission checks when a less-privileged PostgreSQL user is used for the backfill step. Key changes include:
- Modification of the Psycopack constructor in src/psycopack/_repack.py to accept and conditionally bypass permission checks.
- Addition of a new test case in tests/test_repack.py to verify that when skip_permissions_check is set to True, the permissions check is not executed.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| tests/test_repack.py | New test ensuring _check_user_permissions is not called when skip_permissions_check is enabled |
| src/psycopack/_repack.py | Added skip_permissions_check parameter and conditional check to bypass permission validation |
Comments suppressed due to low confidence (1)
src/psycopack/_repack.py:137
- [nitpick] Consider adding documentation for the new 'skip_permissions_check' parameter in the class docstring and init method to clarify its intended usage.
skip_permissions_check: bool = False,
flpcury
approved these changes
Jun 30, 2025
flpcury
left a comment
flpcury
left a comment
There was a problem hiding this comment.
Makes sense to me as a workaround.
On the long-term it would be better to have more granular permission checking (permission to instantiate Psycopack != permission to backfill != permission to sync schemas, etc). That way, there should never be a use case for skipping permission checking
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously, the permissions check was done each time a Psycopack object was instantiated. The check looks for the permissions needed for the complete operation, including all the individual steps. However, one step,
backfill(), doesn't require the much higher elevated permissions that the other steps require. Following the principle of least privilege, it might be desirable to run thebackfill()step using a Postgres user with lower privileges than the user used for the other steps. (Backfilling only does DML operations, whereas most of the other steps do DDL operations that require higher privileges.)This change adds a skip_permissions_check option to Psycopack which enables its use with a less-privileged Postgres user.