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

Implement module to get configuration values from the database #9769

Merged
merged 1 commit into from
Feb 13, 2025

Conversation

mcalmer
Copy link
Contributor

@mcalmer mcalmer commented Feb 13, 2025

What does this PR change?

We started to move configuration values from rhn.conf into the database rhnConfiguration table.
As we need now some of the moved values in python code, we need a function which get us these
from the DB.

GUI diff

No difference.

  • DONE

Documentation

  • No documentation needed: only internal and user invisible changes

  • DONE

Test coverage

  • No tests: add explanation

  • DONE

Links

Addition to: #9423

  • 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!

@mcalmer mcalmer requested a review from a team as a code owner February 13, 2025 09:24
@mcalmer mcalmer requested review from meaksh and removed request for a team February 13, 2025 09:24
@mcalmer mcalmer force-pushed the fix-min_passwd_len-config branch from 6fe25d2 to 4ec18de Compare February 13, 2025 09:29
@mcalmer mcalmer requested a review from Serp1co February 13, 2025 09:31
Copy link
Contributor

@Serp1co Serp1co left a comment

Choose a reason for hiding this comment

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

lgtm

Serp1co
Serp1co previously approved these changes Feb 13, 2025
Copy link
Contributor

@m-czernek m-czernek left a comment

Choose a reason for hiding this comment

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

A few comments while just passing by.

val = convert_table.get(key)(val)
except ValueError:
pass
else:
Copy link
Contributor

Choose a reason for hiding this comment

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

So we want:

  • For specific values, we want specific type defined in the inner dict
  • Otherwise, we want an int,
  • Unless it's a float.
  • But default return from a DB is string (right?), so if it is neither an int or a float, it'll be a string anyways
  • Finally, if it is an empty string, return None

Is that accurate? (I'm asking because this inner method seems confusing to me, and like we should be able to make it simpler than if/else-with-try-except-try-except)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, we try to convert to the real type int, float or string.
There may special keys which always must be converted to a specified type.
Example: there is a password and you decide to use "1234" as password. We want it to be a string, but that mechanism would convert it to a int if we would not have the convert_table.

Copy link
Member

@meaksh meaksh left a comment

Choose a reason for hiding this comment

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

Besides of the comments made by Alex and Marek, this looks good to me.

agraul
agraul previously approved these changes Feb 13, 2025
@mcalmer mcalmer force-pushed the fix-min_passwd_len-config branch from 2cb4cf0 to 25e88a1 Compare February 13, 2025 13:26
@mcalmer mcalmer merged commit 94f3d10 into uyuni-project:master Feb 13, 2025
21 checks passed
@mcalmer mcalmer deleted the fix-min_passwd_len-config branch February 13, 2025 14:08
attempt to convert a string value to the proper type
"""
convert_table = {
"PSW_CHECK_SPECIAL_CHARACTERS": str,
Copy link
Member

Choose a reason for hiding this comment

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

instead of having this in the code, should we add an extra collum to the table to specify who the value should be interpreted?
Like int, string, etc?

Copy link
Contributor

Choose a reason for hiding this comment

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

Speaking for the Java side:
I don't think it's necessary, we have to know the data type beforehand, no dynamic type parsing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants