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.

Sign up for GitHub

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

Jump to bottom

Add PostgreSQL test user with restricted privileges #57802

Merged
merged 9 commits into from
Jun 26, 2024
Merged

Add PostgreSQL test user with restricted privileges #57802

merged 9 commits into from
Jun 26, 2024

Conversation

strk
Copy link
Contributor

@strk strk commented Jun 19, 2024 •
edited
Loading

The script has a section that gives you instructions about setting up the service file if it doesn't work, expecting it to be already setup makes it harder to bootstrap a test environment.

@github-actions github-actions bot added this to the 3.38.0 milestone Jun 19, 2024
@strk strk changed the title Revert "fix testdata_pg.sh to work with "service=qgis_test"" Do not assume qgis_test service is setup at time of invoking testdata_pg.sh Jun 19, 2024
@strk strk marked this pull request as draft June 19, 2024 14:30
@github-actions GitHub Actions
Copy link

github-actions bot commented Jun 19, 2024 •
edited
Loading

🪟 Windows builds ready!

Windows builds of this PR are available for testing here. Debug symbols for this build are available here.

(Built from commit d29221e)

@strk strk changed the title Do not assume qgis_test service is setup at time of invoking testdata_pg.sh Improve PostgreSQL test database bootstrap script testdata_pg.sh Jun 19, 2024
@strk strk changed the title Improve PostgreSQL test database bootstrap script testdata_pg.sh Improve PostgreSQL test database bootstrap scripts Jun 19, 2024
@strk strk marked this pull request as ready for review June 19, 2024 15:24
@strk strk mentioned this pull request Jun 20, 2024
Merged
@strk strk added PostGIS data provider Chore GitHub and other CI infrastructure changes testsuite Issue related to testsuite labels Jun 20, 2024
@strk strk requested a review from jef-n June 20, 2024 07:52
@qgis qgis deleted a comment from github-actions bot Jun 21, 2024
@nyalldawson nyalldawson reopened this Jun 21, 2024
@strk
Copy link
Contributor Author

strk commented Jun 22, 2024

This is now CI-green and ready for review. It's not directly fixing a bug BUT it is a reorganization useful to test user-facing bugs #28835 (PostgreSQL column-restricted INSERT permissions are ignored) and #57809 (Crash releasing pooled PostgreSQL connection when layer table has column-restricted update privilege)

@strk strk changed the title Improve PostgreSQL test database bootstrap scripts Add PostgreSQL test user with restricted privileges Jun 22, 2024
benoitdm-oslandia
benoitdm-oslandia reviewed Jun 24, 2024
View reviewed changes
Copy link
Contributor

@benoitdm-oslandia benoitdm-oslandia left a comment

Choose a reason for hiding this comment

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

I am agree to remove the service=qgis_test from the script but there should be documented somewhere what this script needs to work (a pg configuration, env vars, etc.). Can you add theses explanations? Regards.

tests/testdata/provider/testdata_pg.sh Show resolved Hide resolved
@strk strk closed this Jun 25, 2024
@strk strk reopened this Jun 25, 2024
benoitdm-oslandia
benoitdm-oslandia approved these changes Jun 25, 2024
View reviewed changes
Copy link
Contributor

@benoitdm-oslandia benoitdm-oslandia left a comment

Choose a reason for hiding this comment

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

LGTM!

@strk
Copy link
Contributor Author

strk commented Jun 26, 2024

Are @qgis/qgis-developers the ones to ping to get an effective review ?

@strk strk merged commit 884559f into qgis:master Jun 26, 2024
28 checks passed
@strk strk deleted the fix-testdata-pg branch June 26, 2024 10:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lnicola lnicola lnicola left review comments

@jef-n jef-n jef-n approved these changes

@benoitdm-oslandia benoitdm-oslandia benoitdm-oslandia approved these changes

Assignees
No one assigned
Labels
Chore GitHub and other CI infrastructure changes PostGIS data provider testsuite Issue related to testsuite
Projects
None yet
Milestone
3.38.0
Development

Successfully merging this pull request may close these issues.
5 participants
@strk @jef-n @lnicola @benoitdm-oslandia @nyalldawson