Note: This bug is displayed in read-only format because
the product is no longer active in Red Hat Bugzilla.
RHEL Engineering is moving the tracking of its product development work on RHEL 6 through RHEL 9 to Red Hat Jira (issues.redhat.com). If you're a Red Hat customer, please continue to file support cases via the Red Hat customer portal. If you're not, please head to the "RHEL project" in Red Hat Jira and file new tickets here. Individual Bugzilla bugs in the statuses "NEW", "ASSIGNED", and "POST" are being migrated throughout September 2023. Bugs of Red Hat partners with an assigned Engineering Partner Manager (EPM) are migrated in late September as per pre-agreed dates. Bugs against components "kernel", "kernel-rt", and "kpatch" are only migrated if still in "NEW" or "ASSIGNED". If you cannot log in to RH Jira, please consult article #7032570. That failing, please send an e-mail to the RH Jira admins at rh-issues@redhat.com to troubleshoot your issue as a user management inquiry. The email creates a ServiceNow ticket with Red Hat. Individual Bugzilla bugs that are migrated will be moved to status "CLOSED", resolution "MIGRATED", and set with "MigratedToJIRA" in "Keywords". The link to the successor Jira issue will be found under "Links", have a little "two-footprint" icon next to it, and direct you to the "RHEL project" in Red Hat Jira (issue links are of type "https://issues.redhat.com/browse/RHEL-XXXX", where "X" is a digit). This same link will be available in a blue banner at the top of the page informing you that that bug has been migrated.
R: 1: Similar lines in 3 files
==certlib:52
==factlib:29
==repolib:30
def __init__(self, lock=ActionLock()):
self.lock = lock
def update(self):
lock = self.lock
lock.acquire()
try:
action = UpdateAction()
return action.perform()
finally:
lock.release()
Class CertLib, FactLib, RepoLib are identical. Use only one. Or create one parent and those 3 will inherit from it.
R: 1: Similar lines in 2 files
==managerlib:508
==managerlib_async:58
def filter_pools(self, compatible, overlapping, uninstalled, text):
"""
Return a list of pool hashes, filtered according to the given options.
This method does not actually hit the server, filtering is done in
memory.
"""
pools = self.incompatible_pools.values()
if compatible:
pools = self.compatible_pools.values()
pool_filter = PoolFilter()
# Filter out products that are not installed if necessary:
if uninstalled:
pools = pool_filter.filter_out_installed(pools)
else:
pools = pool_filter.filter_out_uninstalled(pools)
# Do nothing if set to None:
if overlapping:
pools = pool_filter.filter_out_non_overlapping(pools)
elif overlapping == False:
pools = pool_filter.filter_out_overlapping(pools)
# Filter by product name if necessary:
if text:
pools = pool_filter.filter_product_name(pools, text)
return pools
def merge_pools(self, compatible=True, overlapping=None, uninstalled=False,
text=None):
"""
Return a merged view of pools filtered according to the given options.
Pools for the same product will be merged into a MergedPool object.
Overlapping filter by default is None, meaning the pools will not be
filtered at all. Use True to filter out pools which do not overlap,
or False to filter out pools which do.
"""
pools = self.filter_pools(compatible, overlapping, uninstalled, text)
merged_pools = merge_pools(pools)
return merged_pools
R: 1: Similar lines in 2 files
==managerlib:487
==managerlib_async:36
self.all_pools = {}
self.compatible_pools = {}
log.debug("Refreshing pools from server...")
for pool in list_pools(self.backend.uep,
self.consumer.uuid, self.facts, active_on=active_on):
self.compatible_pools[pool['id']] = pool
self.all_pools[pool['id']] = pool
# Filter the list of all pools, removing those we know are compatible.
# Sadly this currently requires a second query to the server.
self.incompatible_pools = {}
for pool in list_pools(self.backend.uep,
self.consumer.uuid, self.facts, all=True, active_on=active_on):
if not pool['id'] in self.compatible_pools:
self.incompatible_pools[pool['id']] = pool
self.all_pools[pool['id']] = pool
log.debug("found %s pools:" % len(self.all_pools))
log.debug(" %s compatible" % len(self.compatible_pools))
log.debug(" %s incompatible" % len(self.incompatible_pools))
There is no need to duplicate so much code of PoolStash class. Create PoolStashBase with common code and in both module change only that part which behave differently.
Main offenders are removed. There is a couple of import dups, but I am not going to worry about that.
Fixed in commit:
7828287a80aa9896ff6cf72b27d3c9327c2584bd
in master.
in subscription-manager git:
find src -name '*.py'|xargs pylint
4.5 % of code is still duplicate
and e.g. class factlib.FactLib and repolib.RepoLib and 100% identical. And it is 75% identical to certlib.CertLib
another example is:
./src/subscription_manager/managercli.py function CliCommand._request_validity_check should use utils.get_dbus_iface instead of its 5 first lines.
Additionally to comment #7 - are you sure you committed your work? Because such commit does not exist:
[msuchy@dri/~/rhn/subscription-manager{master}]$ git pull --rebase
Current branch master is up to date.
[msuchy@dri/~/rhn/subscription-manager{master}]$ git show 7828287a80aa9896ff6cf72b27d3c9327c2584bd
fatal: bad object 7828287a80aa9896ff6cf72b27d3c9327c2584bd
R: 1: Similar lines in 3 files ==certlib:52 ==factlib:29 ==repolib:30 def __init__(self, lock=ActionLock()): self.lock = lock def update(self): lock = self.lock lock.acquire() try: action = UpdateAction() return action.perform() finally: lock.release() Class CertLib, FactLib, RepoLib are identical. Use only one. Or create one parent and those 3 will inherit from it. R: 1: Similar lines in 2 files ==managerlib:508 ==managerlib_async:58 def filter_pools(self, compatible, overlapping, uninstalled, text): """ Return a list of pool hashes, filtered according to the given options. This method does not actually hit the server, filtering is done in memory. """ pools = self.incompatible_pools.values() if compatible: pools = self.compatible_pools.values() pool_filter = PoolFilter() # Filter out products that are not installed if necessary: if uninstalled: pools = pool_filter.filter_out_installed(pools) else: pools = pool_filter.filter_out_uninstalled(pools) # Do nothing if set to None: if overlapping: pools = pool_filter.filter_out_non_overlapping(pools) elif overlapping == False: pools = pool_filter.filter_out_overlapping(pools) # Filter by product name if necessary: if text: pools = pool_filter.filter_product_name(pools, text) return pools def merge_pools(self, compatible=True, overlapping=None, uninstalled=False, text=None): """ Return a merged view of pools filtered according to the given options. Pools for the same product will be merged into a MergedPool object. Overlapping filter by default is None, meaning the pools will not be filtered at all. Use True to filter out pools which do not overlap, or False to filter out pools which do. """ pools = self.filter_pools(compatible, overlapping, uninstalled, text) merged_pools = merge_pools(pools) return merged_pools R: 1: Similar lines in 2 files ==managerlib:487 ==managerlib_async:36 self.all_pools = {} self.compatible_pools = {} log.debug("Refreshing pools from server...") for pool in list_pools(self.backend.uep, self.consumer.uuid, self.facts, active_on=active_on): self.compatible_pools[pool['id']] = pool self.all_pools[pool['id']] = pool # Filter the list of all pools, removing those we know are compatible. # Sadly this currently requires a second query to the server. self.incompatible_pools = {} for pool in list_pools(self.backend.uep, self.consumer.uuid, self.facts, all=True, active_on=active_on): if not pool['id'] in self.compatible_pools: self.incompatible_pools[pool['id']] = pool self.all_pools[pool['id']] = pool log.debug("found %s pools:" % len(self.all_pools)) log.debug(" %s compatible" % len(self.compatible_pools)) log.debug(" %s incompatible" % len(self.incompatible_pools)) There is no need to duplicate so much code of PoolStash class. Create PoolStashBase with common code and in both module change only that part which behave differently.