Skip to content

Commit

Permalink
Removing changes associated with Bugbear rule B019
Browse files Browse the repository at this point in the history
This PR reverts the change from lru_cache to cachedmethod.
The primary goal of the original change was to avoid memory leaks, due
to lru_cache holding instances of a class alive longer than they should
be.
However, for our test framework, the workaround to avoid these memory
leaks really aren't worth it.

I also added an lru_cache to cli_factory.__getattr__, improving its
recurring lookup time substantially.

Finally, I corrected a type check that was erroneously converted in the
initial pass.

(cherry picked from commit 8f7fe94)
  • Loading branch information
JacobCallahan authored and web-flow committed Jan 10, 2024
1 parent 328f1fb commit 44be968
Show file tree
Hide file tree
Showing 4 changed files with 8 additions and 8 deletions.
1 change: 1 addition & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ select = [
]

ignore = [
"B019", # lru_cache can lead to memory leaks - acceptable tradeoff
"E501", # line too long - handled by black
"PT004", # pytest underscrore prefix for non-return fixtures
"PT005", # pytest no underscrore prefix for return fixtures
Expand Down
6 changes: 3 additions & 3 deletions robottelo/host_helpers/cli_factory.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
example: my_satellite.cli_factory.make_org()
"""
import datetime
from functools import partial
from functools import lru_cache, partial
import inspect
import os
from os import chmod
Expand All @@ -14,7 +14,6 @@
from time import sleep

from box import Box
from cachetools import cachedmethod
from fauxfactory import (
gen_alpha,
gen_alphanumeric,
Expand Down Expand Up @@ -250,6 +249,7 @@ def __init__(self, satellite):
self._satellite = satellite
self.__dict__.update(initiate_repo_helpers(self._satellite))

@lru_cache
def __getattr__(self, name):
"""We intercept the usual attribute behavior on this class to emulate make_entity methods
The keys in the dictionary above correspond to potential make_<key> methods
Expand Down Expand Up @@ -295,7 +295,7 @@ def _evaluate_functions(self, iterable):
if not key.startswith('_')
}

@cachedmethod
@lru_cache
def _find_entity_class(self, entity_name):
entity_name = entity_name.replace('_', '').lower()
for name, class_obj in self._satellite.cli.__dict__.items():
Expand Down
4 changes: 2 additions & 2 deletions robottelo/host_helpers/satellite_mixins.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import contextlib
from functools import lru_cache
import io
import os
import random
import re

from cachetools import cachedmethod
import requests

from robottelo.cli.proxy import CapsuleTunnelError
Expand Down Expand Up @@ -357,6 +357,6 @@ def api_factory(self):
self._api_factory = APIFactory(self)
return self._api_factory

@cachedmethod
@lru_cache
def ui_factory(self, session):
return UIFactory(self, session=session)
5 changes: 2 additions & 3 deletions robottelo/hosts.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
from box import Box
from broker import Broker
from broker.hosts import Host
from cachetools import cachedmethod
from dynaconf.vendor.box.exceptions import BoxKeyError
from fauxfactory import gen_alpha, gen_string
from nailgun import entities
Expand Down Expand Up @@ -2170,7 +2169,7 @@ def get_rhsso_client_id(self):
break
return client_id

@cachedmethod
@lru_cache
def get_rhsso_user_details(self, username):
"""Getter method to receive the user id"""
result = self.execute(
Expand All @@ -2179,7 +2178,7 @@ def get_rhsso_user_details(self, username):
result_json = json.loads(result.stdout)
return result_json[0]

@cachedmethod
@lru_cache
def get_rhsso_groups_details(self, group_name):
"""Getter method to receive the group id"""
result = self.execute(f"{KEY_CLOAK_CLI} get groups -r {settings.rhsso.realm}")
Expand Down

0 comments on commit 44be968

Please sign in to comment.