[PATCH v1 2/2] dts: improve starting and stopping interactive shells
Juraj Linkeš
juraj.linkes at pantheon.tech
Thu Jul 11 16:53:28 CEST 2024
With the docs changes below,
Reviewed-by: Juraj Linkeš <juraj.linkes at pantheon.tech>
> diff --git a/dts/framework/remote_session/interactive_shell.py b/dts/framework/remote_session/interactive_shell.py
> index 11dc8a0643..fcaf1f6a5f 100644
> --- a/dts/framework/remote_session/interactive_shell.py
> +++ b/dts/framework/remote_session/interactive_shell.py
> @@ -9,6 +9,9 @@
> collection.
> """
>
> +import weakref
> +from typing import ClassVar
> +
> from .single_active_interactive_shell import SingleActiveInteractiveShell
>
>
> @@ -16,18 +19,26 @@ class InteractiveShell(SingleActiveInteractiveShell):
> """Adds manual start and stop functionality to interactive shells.
>
> Like its super-class, this class should not be instantiated directly and should instead be
> - extended. This class also provides an option for automated cleanup of the application through
> - the garbage collector.
> + extended. This class also provides an option for automated cleanup of the application using a
> + weakref and a finalize class. This finalize class allows for cleanup of the class at the time
> + of garbage collection and also ensures that cleanup only happens once. This way if a user
> + initiates the closing of the shell manually it is not repeated at the time of garbage
> + collection.
> """
>
> + _finalizer: weakref.finalize
> + #: Shells that do not require only one instance to be running shouldn't need more than 1
> + #: attempt to start.
This wording is a bit confusing. This could mean that the shells could
require multiple instances. We could try an alternative approach:
One attempt should be enough for shells which don't have to worry about
other instances closing before starting a new one.
Or something similar.
> + _init_attempts: ClassVar[int] = 1
> +
> def start_application(self) -> None:
> - """Start the application."""
> + """Start the application.
> +
> + After the application has started, add a weakref finalize class to manage cleanup.
I think this could be improved to:
use :class:`weakref.finalize` to manage cleanup
> + """
> self._start_application()
> + self._finalizer = weakref.finalize(self, self._close)
>
> def close(self) -> None:
> - """Properly free all resources."""
> - self._close()
> -
> - def __del__(self) -> None:
> - """Make sure the session is properly closed before deleting the object."""
> - self.close()
> + """Free all resources using finalize class."""
Also here:
using :class:`weakref.finalize`
> + self._finalizer()
More information about the dev
mailing list