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.

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

Do not ignore SQL failures and check that we still work with previous data when changing password #544

Merged
merged 1 commit into from
Jan 9, 2024

Conversation

hhorak
Copy link
Member

@hhorak hhorak commented Jan 4, 2024

Fix also postgresql_cmd function that should not ignore SQL errors (thus using ON_ERROR_STOP option now) and was often called with an interactive input using <<< but podman run is run in non-interactive mode, resulting in the SQL commands to not be printed, so checking them later did not work and errors are not visible. Using -c option solves this.

Function test_postgresql was called with a parameter that was not used anywhere. This function used CREATE EXTENSION to test proper PostgreSQL functionality, that only admin have permissions for.
So, the parameter of test_postgresql function was changed to have a useful meaning. The meaning is that only if the parameter is "admin", the CREATE EXTENSION statement is run to not see
"Not enough permissions" error when the contianer is run as a normal user.

@hhorak
Copy link
Member Author

hhorak commented Jan 4, 2024

[test]

@hhorak hhorak changed the title Check that we still work with previous data when changing password Do not ignore SQL failures and check that we still work with previous data when changing password Jan 5, 2024
… data when changing password

Fix also postgresql_cmd function that should not ignore SQL errors
(thus using ON_ERROR_STOP option now) and was often called with an
interactive input using <<< but podman run is run in non-interactive
mode, resulting in the SQL commands to not be printed, so checking
them later did not work and errors are not visible.
Using -c option solves this.

Function test_postgresql was called with a parameter that was not
used anywhere. This function used CREATE EXTENSION to test proper
PostgreSQL functionality, that only admin have permissions for.
So, the parameter of test_postgresql function was changed to have
a useful meaning. The meaning is that only if the parameter is
"admin", the CREATE EXTENSION statement is run to not see
"Not enough permissions" error when the contianer is run as a
normal user.
@hhorak
Copy link
Member Author

hhorak commented Jan 5, 2024

[test]

Copy link
Contributor

@zmiklank zmiklank left a comment

Choose a reason for hiding this comment

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

Thanks for this PR.

LGTM. I would consider adding '-At -c' params into postgresql_cmd function directly (even though I noticed, that some commands do not use the '-At' params currently), but I am also OK with this solution.

@hhorak
Copy link
Member Author

hhorak commented Jan 9, 2024

LGTM. I would consider adding '-At -c' params into postgresql_cmd function directly (even though I noticed, that some commands do not use the '-At' params currently), but I am also OK with this solution.

I'd rather leave it flexible, so the caller of postgresql_cmd can decide what format and way of input to use.

@hhorak hhorak merged commit cc7b9f2 into sclorg:master Jan 9, 2024
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants