Skip to content

Commit

Permalink
Lazy load the cli.Client machine
Browse files Browse the repository at this point in the history
Problem:

Everytime we instantiate a new instance of `cli.Client` its `__init__` method
code tries to make communication with the remote host to determine if it is
a local or ssh machine to be used.

The communication with `machine` should be done only when it is necessary,
which is when the `run` command is called.

Solution:

Transform `machine` in a lazy `@property` then it is instantiated only
when it is first called.
  • Loading branch information
rochacbruno authored and bherrin3 committed Mar 28, 2019
1 parent 34ac7e2 commit c7a71b1
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 19 deletions.
38 changes: 21 additions & 17 deletions pulp_smash/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -193,26 +193,30 @@ def __init__(self, cfg, response_handler=None, pulp_host=None):
pulp_host = cfg.get_hosts("pulp cli")[0]
else:
pulp_host = cfg.get_hosts("shell")[0]
self.pulp_host = pulp_host
hostname = pulp_host.hostname
transport = pulp_host.roles.get("shell", {}).get("transport")
if transport is None:
transport = "local" if hostname == socket.getfqdn() else "ssh"
if transport == "local":
self.machine = plumbum.machines.local
else: # transport == 'ssh'
# The SshMachine is a wrapper around the host's "ssh" binary.
# Thus, it uses ~/.ssh/config, ~/.ssh/known_hosts, etc.
self.machine = plumbum.machines.SshMachine(hostname)

# How do we handle responses?
if response_handler is None:
self.response_handler = code_handler
else:
self.response_handler = response_handler

self.pulp_host = pulp_host
self.response_handler = response_handler or code_handler
self.cfg = cfg

self._is_root_cache = None
self._machine = None

@property
def machine(self):
"""Initialize the plumbum machine lazily."""
if self._machine is None:
hostname = self.pulp_host.hostname
transport = self.pulp_host.roles.get("shell", {}).get("transport")
if transport is None:
transport = "local" if hostname == socket.getfqdn() else "ssh"

if transport == "local":
self._machine = plumbum.machines.local
else: # transport == 'ssh'
# The SshMachine is a wrapper around the host's "ssh" binary.
# Thus, it uses ~/.ssh/config, ~/.ssh/known_hosts, etc.
self._machine = plumbum.machines.SshMachine(hostname)
return self._machine

@property
def is_superuser(self):
Expand Down
4 changes: 2 additions & 2 deletions tests/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ def test_run(self):
]
)
client = cli.Client(cfg)
with mock.patch.object(client, "machine") as machine:
with mock.patch.object(client, "_machine") as machine:

machine.__getitem__.return_value = machine
machine.run.return_value = (0, "ok", None)
Expand All @@ -205,7 +205,7 @@ def test_run_as_sudo(self):
]
)
client = cli.Client(cfg)
with mock.patch.object(client, "machine") as machine:
with mock.patch.object(client, "_machine") as machine:

machine.__getitem__.return_value = machine
machine.run.return_value = (0, "ok", None)
Expand Down

0 comments on commit c7a71b1

Please sign in to comment.