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 use postgres extensions dblink and pgcrypto #9367

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

nadvornik
Copy link
Contributor

@nadvornik nadvornik commented Oct 15, 2024

What does this PR change?

This PR drops postgres extensions dblink and pgcrypto. This makes use of external database easier.
The extensions seem to be no longer used.

pgcrypto is not used since #8045

dblink is used in function pg_dblink_exec. This function is never called.

GUI diff

No difference.

  • DONE

Documentation

  • No documentation needed: only internal and user invisible changes
  • DONE

Test coverage

  • No tests: already covered

  • DONE

Links

Issue(s): https://github.com/SUSE/spacewalk/issues/25363

  • DONE

Changelogs

Make sure the changelogs entries you are adding are compliant with https://github.com/uyuni-project/uyuni/wiki/Contributing#changelogs and https://github.com/uyuni-project/uyuni/wiki/Contributing#uyuni-projectuyuni-repository

If you don't need a changelog check, please mark this checkbox:

  • No changelog needed

If you uncheck the checkbox after the PR is created, you will need to re-run changelog_test (see below)

Re-run a test

If you need to re-run a test, please mark the related checkbox, it will be unchecked automatically once it has re-run:

  • Re-run test "changelog_test"
  • Re-run test "backend_unittests_pgsql"
  • Re-run test "java_pgsql_tests"
  • Re-run test "schema_migration_test_pgsql"
  • Re-run test "susemanager_unittests"
  • Re-run test "javascript_lint"
  • Re-run test "spacecmd_unittests"

Before you merge

Check How to branch and merge properly!

aaannz
aaannz previously approved these changes Oct 15, 2024
Copy link
Contributor

@aaannz aaannz left a comment

Choose a reason for hiding this comment

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

LGTM, just small nitpick. But I wonder how to approach this. Do we just merge this into uyuni and see what happens, or should we have separate branch for this.

Apparently dblink was used as a simulation of autonomous transaction, whatever it means.

schema/reportdb/postgres/start.sql Outdated Show resolved Hide resolved
mcalmer
mcalmer previously approved these changes Oct 15, 2024
mbussolotto
mbussolotto previously approved these changes Oct 15, 2024
cbosdo
cbosdo previously approved these changes Oct 16, 2024
Comment on lines 32 to 33
if [[ "$s" == *oracle ]]; then
db="oracle"
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't we kick those 2 lines out?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Whole schema/sm-specific-schema-patches seems to be obsolete and can be dropped, IMHO. It has not been touched since 2018. But that is for another PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, this was needed when spacewalk had incompatible changes compared to SUMA and we had to copy them again and again over. We can remove this

schema/spacewalk/postgres/start.sql Show resolved Hide resolved
@cbosdo
Copy link
Contributor

cbosdo commented Oct 16, 2024

I saw no upgrade script... is that intended? How would that fly with upgrades?

@nadvornik
Copy link
Contributor Author

I saw no upgrade script... is that intended? How would that fly with upgrades?

Fixed.

Copy link
Member

@rjmateus rjmateus left a comment

Choose a reason for hiding this comment

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

Just one question, other then that all looks good to me

-- granted to use or replicate Red Hat trademarks that are incorporated
-- in this software or its documentation.

create or replace function pg_dblink_exec(in_sql in varchar) returns void as
Copy link
Member

Choose a reason for hiding this comment

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

Should we also remove this function from the upgrade script?

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, this is needed.

Copy link
Contributor

@cbosdo cbosdo left a comment

Choose a reason for hiding this comment

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

missing a function removal in the update script. The rest LGTM

-- granted to use or replicate Red Hat trademarks that are incorporated
-- in this software or its documentation.

create or replace function pg_dblink_exec(in_sql in varchar) returns void as
Copy link
Contributor

Choose a reason for hiding this comment

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

yes, this is needed.

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.

6 participants