[RHELC-779] Remove unused `call_yum_cmd_w_downgrades function` #669

r0x0d · 2022-11-23T19:33:00Z

After merging the #528, we left behind a few comments to be addressed in a future PR as they were mostly about comments/logs that could be improved.

In this PR, there is also a removal of one unused function that was marked as a TODO in #528.

Signed-off-by: Rodolfo Olivieri rolivier@redhat.com

Jira Issue: RHELC-779

Checklist

PR meets acceptance criteria specified in the Jira issue
PR has been tested manually in a VM (either author or reviewer)
Jira issue has been made public if possible
[RHELC-] is part of the PR title
Code and tests are documented properly
The commits are squashed to as few commits as possible (without losing data)
When merged: Jira issue has been updated to Release Pending

Blocker

Probably it is best to merge this after [RHELC-776] Remove a no longer necessary UEFI-related workaround for Oracle Linux 7 #668 as it removes codes that this PR is intended to remove too.

codecov · 2022-11-23T19:34:49Z

Codecov Report

Base: 92.44% // Head: 92.46% // Increases project coverage by +0.01% 🎉

Coverage data is based on head (992fc06) compared to base (d76665d).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #669      +/-   ##
==========================================
+ Coverage   92.44%   92.46%   +0.01%     
==========================================
  Files          22       22              
  Lines        3192     3171      -21     
  Branches      564      559       -5     
==========================================
- Hits         2951     2932      -19     
+ Misses        173      172       -1     
+ Partials       68       67       -1

Flag	Coverage Δ
centos-linux-7	`89.21% <50.00%> (-0.01%)`	⬇️
centos-linux-8	`89.40% <50.00%> (-0.01%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
convert2rhel/pkghandler.py	`92.80% <ø> (+0.10%)`	⬆️
convert2rhel/pkgmanager/handlers/dnf.py	`100.00% <100.00%> (ø)`
convert2rhel/pkgmanager/handlers/yum.py	`97.87% <100.00%> (ø)`

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

r0x0d · 2022-11-23T19:37:20Z

@danmyway while removing away old code that was not necessary any more, I found out this test https://github.com/oamg/convert2rhel/blob/leftover-comments-from-merge-yum-transactions/tests/integration/tier1/yum-distro-sync/test_yum_distro_sync.py#L13 that seems to mimic the function that I removed, the call_yum_cmd_w_downgrades.

That function was replaced with the {dnf,yum}.run_transaction() in the PR #528.

Do you think it's necessary to still have this integration test or adapt it somehow to be more related to the ones defined here https://github.com/oamg/convert2rhel/blob/a53baba4d2fc445b32b051eb782e5aef1c2c8768/tests/integration/tier1/single-yum-transaction/test_single_yum_transaction.py?

r0x0d · 2022-11-23T19:39:07Z

convert2rhel/pkghandler.py

@@ -67,49 +67,6 @@ def __init__(self, pkg_obj, fingerprint):
        self.fingerprint = fingerprint


-def call_yum_cmd_w_downgrades(cmd, pkgs, retries=MAX_YUM_CMD_CALLS):


That function was not being used anywhere else other than in unit tests.

danmyway · 2022-11-30T13:53:56Z

@danmyway while removing away old code that was not necessary any more, I found out this test https://github.com/oamg/convert2rhel/blob/leftover-comments-from-merge-yum-transactions/tests/integration/tier1/yum-distro-sync/test_yum_distro_sync.py#L13 that seems to mimic the function that I removed, the call_yum_cmd_w_downgrades.

That function was replaced with the {dnf,yum}.run_transaction() in the PR #528.

Do you think it's necessary to still have this integration test or adapt it somehow to be more related to the ones defined here https://github.com/oamg/convert2rhel/blob/a53baba4d2fc445b32b051eb782e5aef1c2c8768/tests/integration/tier1/single-yum-transaction/test_single_yum_transaction.py?

IIUIC, I believe, that we do not need the integration test then, @r0x0d

abadger

I know this is still going to get some work as Rodolfo handles the integration test update but it looks good so far. It hits all of the acceptance criteria specified i nthe jira ticket.

r0x0d · 2022-11-30T17:51:06Z

@danmyway while removing away old code that was not necessary any more, I found out this test https://github.com/oamg/convert2rhel/blob/leftover-comments-from-merge-yum-transactions/tests/integration/tier1/yum-distro-sync/test_yum_distro_sync.py#L13 that seems to mimic the function that I removed, the call_yum_cmd_w_downgrades.
That function was replaced with the {dnf,yum}.run_transaction() in the PR #528.
Do you think it's necessary to still have this integration test or adapt it somehow to be more related to the ones defined here https://github.com/oamg/convert2rhel/blob/a53baba4d2fc445b32b051eb782e5aef1c2c8768/tests/integration/tier1/single-yum-transaction/test_single_yum_transaction.py?

IIUIC, I believe, that we do not need the integration test then, @r0x0d

I will leave it here for a while. We can come back to this after the release and remove the integration test if it is not needed any more.

tests/integration/tier1/handle-special-pkgs-case/test_handle_shim_x64_pkg.py

convert2rhel/special_cases.py

tests/integration/tier1/handle-special-pkgs-case/test_handle_shim_x64_pkg.py

convert2rhel/special_cases.py

r0x0d · 2023-01-23T12:35:07Z

/packit test

Venefilyn · 2023-02-06T16:59:03Z

What is this blocked on? Jira issue doesn't say

r0x0d · 2023-02-06T17:02:18Z

We were blocked by #668, but it's merged now. Just forgot to remove the label

abadger

This is a good change. +1 to merge.

After merging the oamg#528, we left behind a few comments to be addressed in a future PR as they were mostly about comments/logs that could be improved. In this PR, there is also a removal of one unused function that was marked as a TODO in oamg#528. Signed-off-by: Rodolfo Olivieri <rolivier@redhat.com>

Signed-off-by: Rodolfo Olivieri <rolivier@redhat.com>

r0x0d · 2023-02-08T13:16:01Z

Rebased to trigger tests, let's see if the failures are only related to things that are not part from this PR.

Venefilyn · 2023-02-08T17:27:28Z

Couldn't see anything with tests that isn't caused by other things. Merging and monitoring main branch results

bocekm · 2023-02-17T18:17:13Z

convert2rhel/pkghandler.py

-    output = resolve_dep_errors(output)
-
-    # if we have problematic packages, remove them
-    problematic_pkgs = get_problematic_pkgs(output)


These two functions, resolve_dep_errors() and get_problematic_pkgs(), are not used anywhere else and might have been removed together with the call_yum_cmd_w_downgrades().
I'll create a ticket to have that done. EDIT: Here it is: https://issues.redhat.com/browse/RHELC-903.

Some dependencies related to dependency error resolution were left behind while working on oamg#669. This commit introduces the removal of the functions that were being left out. Signed-off-by: Rodolfo Olivieri <rolivier@redhat.com>

* Remove unused dependency error resolution functions Some dependencies related to dependency error resolution were left behind while working on #669. This commit introduces the removal of the functions that were being left out. Signed-off-by: Rodolfo Olivieri <rolivier@redhat.com> * Remove extra constants Some contants were left in the code after removing some tests in pkghandler_test.py and in pkghandler.py. Signed-off-by: Rodolfo Olivieri <rolivier@redhat.com> * Remove extra constant and unused file Signed-off-by: Rodolfo Olivieri <rolivier@redhat.com> --------- Signed-off-by: Rodolfo Olivieri <rolivier@redhat.com>

r0x0d self-assigned this Nov 23, 2022

r0x0d force-pushed the leftover-comments-from-merge-yum-transactions branch from 286665a to b2bc732 Compare November 23, 2022 19:34

r0x0d changed the title ~~Fix leftover comments from #528~~ RHELC-779 Fix leftover comments from #528 Nov 23, 2022

r0x0d changed the title ~~RHELC-779 Fix leftover comments from #528~~ [RHELC-779] Fix leftover comments from #528 Nov 23, 2022

r0x0d commented Nov 23, 2022

View reviewed changes

abadger previously approved these changes Nov 30, 2022

View reviewed changes

r0x0d added the blocked label Nov 30, 2022

Venefilyn added the enhancement New feature or request label Dec 13, 2022

r0x0d dismissed abadger’s stale review via 5cca81d January 2, 2023 18:36

r0x0d force-pushed the leftover-comments-from-merge-yum-transactions branch from b2bc732 to 5cca81d Compare January 2, 2023 18:36

Venefilyn reviewed Jan 9, 2023

View reviewed changes

tests/integration/tier1/handle-special-pkgs-case/test_handle_shim_x64_pkg.py Outdated Show resolved Hide resolved

convert2rhel/special_cases.py Outdated Show resolved Hide resolved

Venefilyn changed the title ~~[RHELC-779] Fix leftover comments from #528~~ [RHELC-779] Remove unused function call_yum_cmd_w_downgrades Jan 9, 2023

r0x0d commented Jan 9, 2023

View reviewed changes

tests/integration/tier1/handle-special-pkgs-case/test_handle_shim_x64_pkg.py Outdated Show resolved Hide resolved

r0x0d commented Jan 9, 2023

View reviewed changes

convert2rhel/special_cases.py Outdated Show resolved Hide resolved

r0x0d force-pushed the leftover-comments-from-merge-yum-transactions branch from 4be813a to 2838533 Compare February 6, 2023 16:25

r0x0d removed the blocked label Feb 6, 2023

abadger previously approved these changes Feb 7, 2023

View reviewed changes

r0x0d added 3 commits February 8, 2023 10:15

Fix CodeQL warning in unit_tests

7a7fc76

Signed-off-by: Rodolfo Olivieri <rolivier@redhat.com>

Remove empty file left from rebase

992fc06

Signed-off-by: Rodolfo Olivieri <rolivier@redhat.com>

r0x0d dismissed abadger’s stale review via 992fc06 February 8, 2023 13:15

r0x0d force-pushed the leftover-comments-from-merge-yum-transactions branch from 3f6a481 to 992fc06 Compare February 8, 2023 13:15

abadger approved these changes Feb 8, 2023

View reviewed changes

Venefilyn merged commit 0b261dc into oamg:main Feb 8, 2023

r0x0d deleted the leftover-comments-from-merge-yum-transactions branch February 8, 2023 17:31

bocekm reviewed Feb 17, 2023

View reviewed changes

Venefilyn mentioned this pull request Feb 20, 2023

Bump version to 1.2 #742

Merged

Venefilyn changed the title ~~[RHELC-779] Remove unused function call_yum_cmd_w_downgrades~~ [RHELC-779] Remove unused call_yum_cmd_w_downgrades function Feb 22, 2023

r0x0d mentioned this pull request May 8, 2023

[RHELC-903] Remove unused dependency error resolution functions #822

Merged

8 tasks

Venefilyn pushed a commit that referenced this pull request Jun 19, 2023

[RHELC-779] Remove unused function call_yum_cmd_w_downgrades (#669)

0e1ef0f

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[RHELC-779] Remove unused `call_yum_cmd_w_downgrades function` #669

[RHELC-779] Remove unused `call_yum_cmd_w_downgrades function` #669

r0x0d commented Nov 23, 2022 •

edited by bocekm

codecov bot commented Nov 23, 2022 •

edited

r0x0d commented Nov 23, 2022

r0x0d Nov 23, 2022

danmyway commented Nov 30, 2022

abadger left a comment

r0x0d commented Nov 30, 2022

r0x0d commented Jan 23, 2023

Venefilyn commented Feb 6, 2023

r0x0d commented Feb 6, 2023

abadger left a comment •

edited

r0x0d commented Feb 8, 2023

Venefilyn commented Feb 8, 2023

bocekm Feb 17, 2023 •

edited

		@@ -67,49 +67,6 @@ def __init__(self, pkg_obj, fingerprint):
		self.fingerprint = fingerprint


		def call_yum_cmd_w_downgrades(cmd, pkgs, retries=MAX_YUM_CMD_CALLS):

[RHELC-779] Remove unused call_yum_cmd_w_downgrades function #669

[RHELC-779] Remove unused call_yum_cmd_w_downgrades function #669

Conversation

r0x0d commented Nov 23, 2022 • edited by bocekm

Blocker

codecov bot commented Nov 23, 2022 • edited

Codecov Report

r0x0d commented Nov 23, 2022

r0x0d Nov 23, 2022

Choose a reason for hiding this comment

danmyway commented Nov 30, 2022

abadger left a comment

Choose a reason for hiding this comment

r0x0d commented Nov 30, 2022

r0x0d commented Jan 23, 2023

Venefilyn commented Feb 6, 2023

r0x0d commented Feb 6, 2023

abadger left a comment • edited

Choose a reason for hiding this comment

r0x0d commented Feb 8, 2023

Venefilyn commented Feb 8, 2023

bocekm Feb 17, 2023 • edited

Choose a reason for hiding this comment

[RHELC-779] Remove unused `call_yum_cmd_w_downgrades function` #669

[RHELC-779] Remove unused `call_yum_cmd_w_downgrades function` #669

r0x0d commented Nov 23, 2022 •

edited by bocekm

codecov bot commented Nov 23, 2022 •

edited

abadger left a comment •

edited

bocekm Feb 17, 2023 •

edited