1
0
mirror of https://github.com/dkmstr/openuds.git synced 2024-12-22 13:34:04 +03:00

The purpose of these changes was primarily to enhance code readability and maintenance, and secondly to improve performance through caching.

In `userservice.py`, a change was made on how `can_grow_service_pool` method's result is evaluated, from explicitly comparing it with `False` to a pythonic approach of using `not`. This makes the code cleaner and easier to read.

In `service.py` file, a variable name was changed from `services_limit` to `userservices_limit_field`. It appears much more logical namewise as the variable deals with limiting the number of user services.

In the `fixed/service.py` file, the code was refactored for readability and DRY (Don't Repeat Yourself) principle, avoiding code repetition by extracting a recurring pattern into a separate method called `_get_machines_field`.

In `authenticator.py`, a caching mechanism was introduced. If an Authenticator object is already created and no update in values is needed, the existing Authenticator object is returned. This prevents unnecessary creation of new Authenticator objects, thereby improving performance.

In the `managed_object_model.py` file, the instruction to clear the cache was removed. This coincides with the caching strategy introduced in `authenticator.py`.

Lastly, in `OpenGnsys/service.py`, `services_limit` was renamed to `userservices_limit_field`, consistent with the changes in `service.py`, improving code consistency.
This commit is contained in:
Adolfo Gómez García 2024-10-02 21:19:16 +02:00
parent 4a68ebfb6d
commit 1c388f62c1
No known key found for this signature in database
GPG Key ID: DD1ABF20724CDA23
6 changed files with 32 additions and 27 deletions

View File

@ -204,7 +204,7 @@ class UserServiceManager(metaclass=singleton.Singleton):
Creates a new assigned deployed service for the current publication (if any) of service pool and user indicated Creates a new assigned deployed service for the current publication (if any) of service pool and user indicated
""" """
# First, honor concurrent_creation_limit # First, honor concurrent_creation_limit
if self.can_grow_service_pool(service_pool) is False: if not self.can_grow_service_pool(service_pool):
# Cannot create new # Cannot create new
operations_logger.info( operations_logger.info(
'Too many preparing services. Creation of assigned service denied by max preparing services parameter. (login storm with insufficient cache?).' 'Too many preparing services. Creation of assigned service denied by max preparing services parameter. (login storm with insufficient cache?).'

View File

@ -163,6 +163,9 @@ class FixedService(services.Service, abc.ABC): # pylint: disable=too-many-publi
if machines != initial_machines: if machines != initial_machines:
d['vms'] = machines # Store it d['vms'] = machines # Store it
def _get_machines_field(self) -> gui.MultiChoiceField:
return typing.cast(gui.MultiChoiceField, getattr(self, self.alternate_machines_field or 'machines'))
def snapshot_creation(self, userservice_instance: 'FixedUserService') -> None: def snapshot_creation(self, userservice_instance: 'FixedUserService') -> None:
""" """
Creates a snapshot for the machine Creates a snapshot for the machine
@ -178,7 +181,7 @@ class FixedService(services.Service, abc.ABC): # pylint: disable=too-many-publi
def unmarshal(self, data: bytes) -> None: def unmarshal(self, data: bytes) -> None:
super().unmarshal(data) super().unmarshal(data)
# Recover userservice # Recover userservice
self.userservices_limit = len(self.machines.as_list()) self.userservices_limit = len(self._get_machines_field().as_list())
@abc.abstractmethod @abc.abstractmethod
def get_name(self, vmid: str) -> str: def get_name(self, vmid: str) -> str:
@ -252,7 +255,7 @@ class FixedService(services.Service, abc.ABC): # pylint: disable=too-many-publi
Default implementation NEEDS machines field to be present!! Default implementation NEEDS machines field to be present!!
""" """
fixed_instance = typing.cast('FixedUserService', userservice_instance) fixed_instance = typing.cast('FixedUserService', userservice_instance)
machines = typing.cast(gui.MultiChoiceField, getattr(self, self.alternate_machines_field or 'machines' )) machines = self._get_machines_field()
with self._assigned_access() as assigned_vms: with self._assigned_access() as assigned_vms:
if assignable_id not in assigned_vms and assignable_id in machines.as_list(): if assignable_id not in assigned_vms and assignable_id in machines.as_list():
assigned_vms.add(assignable_id) assigned_vms.add(assignable_id)
@ -264,12 +267,10 @@ class FixedService(services.Service, abc.ABC): # pylint: disable=too-many-publi
""" """
Randomizes the assignation of machines if needed Randomizes the assignation of machines if needed
""" """
fld_name = self.alternate_machines_field or 'machines' machines = self._get_machines_field()
if self.has_field(fld_name) is False:
raise ValueError(f'machines field {fld_name} not found')
machines = typing.cast(gui.MultiChoiceField, getattr(self, fld_name))
if hasattr(self, 'randomize') and self.randomize.value is True: if hasattr(self, 'randomize') and self.randomize.value is True:
# Randomize the list
return random.sample(machines.as_list(), len(machines.as_list())) return random.sample(machines.as_list(), len(machines.as_list()))
return machines.as_list() return machines.as_list()

View File

@ -120,11 +120,11 @@ class Service(Module):
# : Normally set to UNLIMITED. This attribute indicates if the service has some "limitation" # : Normally set to UNLIMITED. This attribute indicates if the service has some "limitation"
# : for providing user services. This attribute can be set here or # : for providing user services. This attribute can be set here or
# : modified at instance level, core will access always to it using an instance object. # : modified at instance level, core will access always to it using an instance object.
# : Note: you can override this value on service instantiation by providing a "services_limit": # : Note: you can override this value on service instantiation by providing a "userservices_limit_field":
# : - If services_limit is an integer, it will be used as userservices_limit # : - If userservices_limit_field is an integer, it will be used as userservices_limit
# : - If services_limit is a gui.NumericField, it will be used as userservices_limit (.num() will be called) # : - If userservices_limit_field is a gui.NumericField, it will be used as userservices_limit (.num() will be called)
# : - If services_limit is a callable, it will be called and the result will be used as userservices_limit # : - If userservices_limit_field is a callable, it will be called and the result will be used as userservices_limit
# : - If services_limit is None, userservices_limit will be set to consts.UNLIMITED (as default) # : - If userservices_limit_field is None, userservices_limit will be set to consts.UNLIMITED (as default)
userservices_limit: int = consts.UNLIMITED userservices_limit: int = consts.UNLIMITED
# : If this item "has overrided fields", on deployed service edition, defined keys will overwrite defined ones # : If this item "has overrided fields", on deployed service edition, defined keys will overwrite defined ones
@ -278,23 +278,23 @@ class Service(Module):
def unmarshal(self, data: bytes) -> None: def unmarshal(self, data: bytes) -> None:
# In fact, we will not unmarshal anything here, but setup maxDeployed # In fact, we will not unmarshal anything here, but setup maxDeployed
# if services_limit exists and it is a gui.NumericField # if userservices_limit_field exists and it is a gui.NumericField
# Invoke base unmarshal, so "gui fields" gets loaded from data # Invoke base unmarshal, so "gui fields" gets loaded from data
super().unmarshal(data) super().unmarshal(data)
if hasattr(self, 'services_limit'): if hasattr(self, 'userservices_limit_field'):
# Fix self "userservices_limit" value after loading fields # Fix self "userservices_limit" value after loading fields
try: try:
services_limit = getattr(self, 'services_limit', None) userservices_limit_field = getattr(self, 'userservices_limit_field', None)
if isinstance(services_limit, int): if isinstance(userservices_limit_field, int):
self.userservices_limit = services_limit self.userservices_limit = userservices_limit_field
elif isinstance(services_limit, gui.NumericField): elif isinstance(userservices_limit_field, gui.NumericField):
self.userservices_limit = services_limit.as_int() self.userservices_limit = userservices_limit_field.as_int()
# For 0 values on userservices_limit field, we will set it to UNLIMITED # For 0 values on userservices_limit field, we will set it to UNLIMITED
if self.userservices_limit == 0: if self.userservices_limit == 0:
self.userservices_limit = consts.UNLIMITED self.userservices_limit = consts.UNLIMITED
elif callable(services_limit): elif callable(userservices_limit_field):
self.userservices_limit = typing.cast(collections.abc.Callable[..., int], services_limit)() self.userservices_limit = typing.cast(collections.abc.Callable[..., int], userservices_limit_field)()
else: else:
self.userservices_limit = consts.UNLIMITED self.userservices_limit = consts.UNLIMITED
except Exception: except Exception:
@ -304,7 +304,7 @@ class Service(Module):
if self.userservices_limit < 0: if self.userservices_limit < 0:
self.userservices_limit = consts.UNLIMITED self.userservices_limit = consts.UNLIMITED
# Keep untouched if services_limit is not present # Keep untouched if userservices_limit_field is not present
def mac_generator(self) -> 'UniqueMacGenerator': def mac_generator(self) -> 'UniqueMacGenerator':
""" """

View File

@ -102,16 +102,22 @@ class Authenticator(ManagedObjectModel, TaggingMixin):
Raises: Raises:
""" """
if self._cached_instance and values is None:
return typing.cast(auths.Authenticator, self._cached_instance)
if not self.id: if not self.id:
# Return a fake authenticator # Return a fake authenticator
return auths.Authenticator( return auths.Authenticator(
environment.Environment.environment_for_table_record('fake_auth'), values, uuid=self.uuid environment.Environment.environment_for_table_record('fake_auth'), values, uuid=self.uuid
) )
auType = self.get_type() auth_type = self.get_type()
env = self.get_environment() env = self.get_environment()
auth = auType(env, values, uuid=self.uuid) auth = auth_type(env, values, uuid=self.uuid)
self.deserialize(auth, values) self.deserialize(auth, values)
self._cached_instance = auth
return auth return auth
def get_type(self) -> type[auths.Authenticator]: def get_type(self) -> type[auths.Authenticator]:

View File

@ -88,8 +88,6 @@ class ManagedObjectModel(UUIDModel):
self.save(update_fields=['data']) self.save(update_fields=['data'])
obj.mark_for_upgrade(False) obj.mark_for_upgrade(False)
self._cached_instance = None # Ensures returns correct value on get_instance
def get_instance(self, values: typing.Optional[dict[str, str]] = None) -> 'Module': def get_instance(self, values: typing.Optional[dict[str, str]] = None) -> 'Module':
""" """
Instantiates the object this record contains. Instantiates the object this record contains.

View File

@ -143,7 +143,7 @@ class OGService(services.Service):
old_field_name='startIfUnavailable', old_field_name='startIfUnavailable',
) )
services_limit = fields.services_limit_field() userservices_limit_field = fields.services_limit_field()
prov_uuid = gui.HiddenField(value=None) prov_uuid = gui.HiddenField(value=None)