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

Fix error in 'kickstart_delete' when using wildcards (bsc#1227578) #9103

Merged
merged 1 commit into from
Sep 24, 2024

Conversation

cbbayburt
Copy link
Contributor

@cbbayburt cbbayburt commented Jul 25, 2024

In kickstart_delete, the code first resolves wildcard arguments and returns matching labels. Then it tries to make sure if every arg actually matched a result. But since the original wildcard args don't literally match the resulting labels, the second check throws an error. This PR removes the second check (only exists in this method) to match the method with the rest of the system.

See bug: https://bugzilla.suse.com/show_bug.cgi?id=1227578

Documentation

  • No documentation needed: Bugfix

Test coverage

  • Unit tests were added

Links

Fixes https://github.com/SUSE/spacewalk/issues/24761

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!

@cbbayburt cbbayburt requested a review from a team as a code owner July 25, 2024 10:42
@cbbayburt cbbayburt requested review from meaksh and removed request for a team July 25, 2024 10:42
@@ -374,9 +374,9 @@ def test_kickstart_delete_invalid_profile(self, shell):
assert_args_expect(shell.do_kickstart_list.call_args_list,
[(('', True), {})])

def test_kickstart_delete_some_invalid_profile(self, shell):
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't we keep this test?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, this one tests for the case I removed in the code. Basically, if some of the args are not found, it used to throw an error, which didn't work with wildcards. Now it works with only the args it could match. The removed part doesn't exist in any other methods that deal with the globs in the same way, so I removed it from this as well.

Copy link
Member

Choose a reason for hiding this comment

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

How does the user know that some of the passed profiles were not found after your change?

Copy link
Contributor Author

@cbbayburt cbbayburt Jul 25, 2024

Choose a reason for hiding this comment

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

User needs to confirm y/n for every matched label one by one.

Plus, deleted labels are displayed in the output in the end.

@cbbayburt cbbayburt added the merge-candidate Meaning it needs to be considered for merging when the master branch is frozen label Sep 23, 2024
@deneb-alpha deneb-alpha merged commit 792c58e into uyuni-project:master Sep 24, 2024
24 checks passed
@cbbayburt cbbayburt deleted the bsc1227578 branch September 25, 2024 10:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge-candidate Meaning it needs to be considered for merging when the master branch is frozen needs-translation python3
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants