From 5aa36c1861a6972b49df1b6022aa2f4ac30e1e66 Mon Sep 17 00:00:00 2001 From: Jhon Honce Date: Wed, 25 Jul 2018 14:04:34 -0700 Subject: Clean up pylint warnings and errors for podman * Some of the pedantic errors were not corrected * Clean up prep for porting to MacOS and PyPi hosting * Fix broken unittest Signed-off-by: Jhon Honce Closes: #1159 Approved by: rhatdan --- contrib/python/podman/podman/__init__.py | 2 +- contrib/python/podman/podman/client.py | 23 +++++------ contrib/python/podman/podman/libs/__init__.py | 9 +++-- .../podman/podman/libs/_containers_attach.py | 4 +- .../python/podman/podman/libs/_containers_start.py | 11 +++--- contrib/python/podman/podman/libs/containers.py | 44 +++++++++++----------- contrib/python/podman/podman/libs/errors.py | 4 +- contrib/python/podman/podman/libs/images.py | 40 ++++++++++---------- contrib/python/podman/podman/libs/system.py | 6 +-- contrib/python/podman/podman/libs/tunnel.py | 12 +++--- contrib/python/podman/test/test_images.py | 2 +- 11 files changed, 79 insertions(+), 78 deletions(-) (limited to 'contrib') diff --git a/contrib/python/podman/podman/__init__.py b/contrib/python/podman/podman/__init__.py index ec4775178..3b083f007 100644 --- a/contrib/python/podman/podman/__init__.py +++ b/contrib/python/podman/podman/__init__.py @@ -8,7 +8,7 @@ from .libs.errors import (ContainerNotFound, ErrorOccurred, ImageNotFound, try: __version__ = pkg_resources.get_distribution('podman').version -except Exception: +except Exception: # pylint: disable=broad-except __version__ = '0.0.0' __all__ = [ diff --git a/contrib/python/podman/podman/client.py b/contrib/python/podman/podman/client.py index 404b7d117..5efd2be2b 100644 --- a/contrib/python/podman/podman/client.py +++ b/contrib/python/podman/podman/client.py @@ -14,9 +14,15 @@ from .libs.system import System from .libs.tunnel import Context, Portal, Tunnel -class BaseClient(object): +class BaseClient(): """Context manager for API workers to access varlink.""" + def __init__(self, context): + """Construct Client.""" + self._client = None + self._iface = None + self._context = context + def __call__(self): """Support being called for old API.""" return self @@ -74,18 +80,13 @@ class BaseClient(object): remote.hostname, kwargs.get('identity_file'), )) - else: - return LocalClient( - Context(uri, interface, None, None, None, None, None)) + return LocalClient( + Context(uri, interface, None, None, None, None, None)) class LocalClient(BaseClient): """Context manager for API workers to access varlink.""" - def __init__(self, context): - """Construct LocalClient.""" - self._context = context - def __enter__(self): """Enter context for LocalClient.""" self._client = VarlinkClient(address=self._context.uri) @@ -107,7 +108,7 @@ class RemoteClient(BaseClient): def __init__(self, context): """Construct RemoteCLient.""" - self._context = context + super().__init__(context) self._portal = Portal() def __enter__(self): @@ -136,7 +137,7 @@ class RemoteClient(BaseClient): raise error_factory(e) -class Client(object): +class Client(): """A client for communicating with a Podman varlink service. Example: @@ -184,7 +185,7 @@ class Client(object): def __exit__(self, exc_type, exc_value, traceback): """Raise any exception triggered within the runtime context.""" - return None + pass @cached_property def system(self): diff --git a/contrib/python/podman/podman/libs/__init__.py b/contrib/python/podman/podman/libs/__init__.py index 3a8a35021..e9859bee5 100644 --- a/contrib/python/podman/podman/libs/__init__.py +++ b/contrib/python/podman/podman/libs/__init__.py @@ -47,8 +47,9 @@ def datetime_format(dt): """Format datetime in consistent style.""" if isinstance(dt, str): return datetime_parse(dt).isoformat() - elif isinstance(dt, datetime.datetime): + + if isinstance(dt, datetime.datetime): return dt.isoformat() - else: - raise ValueError('Unable to format {}. Type {} not supported.'.format( - dt, type(dt))) + + raise ValueError('Unable to format {}. Type {} not supported.'.format( + dt, type(dt))) diff --git a/contrib/python/podman/podman/libs/_containers_attach.py b/contrib/python/podman/podman/libs/_containers_attach.py index df12fa998..f2dad573b 100644 --- a/contrib/python/podman/podman/libs/_containers_attach.py +++ b/contrib/python/podman/podman/libs/_containers_attach.py @@ -53,8 +53,8 @@ class Mixin: packed = fcntl.ioctl(self.pseudo_tty.stdout, termios.TIOCGWINSZ, struct.pack('HHHH', 0, 0, 0, 0)) rows, cols, _, _ = struct.unpack('HHHH', packed) - logging.debug('Resize window({}x{}) using {}'.format( - rows, cols, self.pseudo_tty.control_socket)) + logging.debug('Resize window(%dx%d) using %s', rows, cols, + self.pseudo_tty.control_socket) # TODO: Need some kind of timeout in case pipe is blocked with open(self.pseudo_tty.control_socket, 'w') as skt: diff --git a/contrib/python/podman/podman/libs/_containers_start.py b/contrib/python/podman/podman/libs/_containers_start.py index ad9f32eab..8e705771a 100644 --- a/contrib/python/podman/podman/libs/_containers_start.py +++ b/contrib/python/podman/podman/libs/_containers_start.py @@ -20,15 +20,14 @@ class Mixin: Will block if container has been detached. """ with self._client() as podman: - results = podman.StartContainer(self.id) - logging.debug('Started Container "{}"'.format( - results['container'])) + results = podman.StartContainer(self._id) + logging.debug('Started Container "%s"', results['container']) if not hasattr(self, 'pseudo_tty') or self.pseudo_tty is None: return self._refresh(podman) - logging.debug('Setting up PseudoTTY for Container "{}"'.format( - results['container'])) + logging.debug('Setting up PseudoTTY for Container "%s"', + results['container']) try: # save off the old settings for terminal @@ -54,7 +53,7 @@ class Mixin: sources = [skt, self.pseudo_tty.stdin] while sources: - logging.debug('Waiting on sources: {}'.format(sources)) + logging.debug('Waiting on sources: %s', sources) readable, _, _ = select.select(sources, [], []) if skt in readable: diff --git a/contrib/python/podman/podman/libs/containers.py b/contrib/python/podman/podman/libs/containers.py index 6dc2c141e..88d2aa137 100644 --- a/contrib/python/podman/podman/libs/containers.py +++ b/contrib/python/podman/podman/libs/containers.py @@ -23,9 +23,9 @@ class Container(AttachMixin, StartMixin, collections.UserDict): with client() as podman: self._refresh(podman) - assert self._id == self.data['id'],\ + assert self._id == data['id'],\ 'Requested container id({}) does not match store id({})'.format( - self._id, self.id + self._id, data['id'] ) def __getitem__(self, key): @@ -52,13 +52,13 @@ class Container(AttachMixin, StartMixin, collections.UserDict): def processes(self): """Show processes running in container.""" with self._client() as podman: - results = podman.ListContainerProcesses(self.id) + results = podman.ListContainerProcesses(self._id) yield from results['container'] def changes(self): """Retrieve container changes.""" with self._client() as podman: - results = podman.ListContainerChanges(self.id) + results = podman.ListContainerChanges(self._id) return results['container'] def kill(self, signal=signal.SIGTERM, wait=25): @@ -68,7 +68,7 @@ class Container(AttachMixin, StartMixin, collections.UserDict): wait n of seconds, 0 waits forever. """ with self._client() as podman: - podman.KillContainer(self.id, signal) + podman.KillContainer(self._id, signal) timeout = time.time() + wait while True: self._refresh(podman) @@ -84,15 +84,15 @@ class Container(AttachMixin, StartMixin, collections.UserDict): """Convert all keys to lowercase.""" @functools.wraps(self._lower_hook) - def wrapped(input): - return {k.lower(): v for (k, v) in input.items()} + def wrapped(input_): + return {k.lower(): v for (k, v) in input_.items()} return wrapped def inspect(self): """Retrieve details about containers.""" with self._client() as podman: - results = podman.InspectContainer(self.id) + results = podman.InspectContainer(self._id) obj = json.loads(results['container'], object_hook=self._lower_hook()) return collections.namedtuple('ContainerInspect', obj.keys())(**obj) @@ -102,7 +102,7 @@ class Container(AttachMixin, StartMixin, collections.UserDict): TODO: should there be a compress option, like images? """ with self._client() as podman: - results = podman.ExportContainer(self.id, target) + results = podman.ExportContainer(self._id, target) return results['tarfile'] def commit(self, @@ -130,7 +130,7 @@ class Container(AttachMixin, StartMixin, collections.UserDict): # TODO: Clean up *args, **kwargs after Commit() is complete try: author = kwargs.get('author', getpass.getuser()) - except Exception: + except Exception: # pylint: disable=broad-except author = '' for c in changes: @@ -140,14 +140,14 @@ class Container(AttachMixin, StartMixin, collections.UserDict): format(c)) with self._client() as podman: - results = podman.Commit(self.id, image_name, changes, author, + results = podman.Commit(self._id, image_name, changes, author, message, pause) return results['image'] def stop(self, timeout=25): """Stop container, return id on success.""" with self._client() as podman: - podman.StopContainer(self.id, timeout) + podman.StopContainer(self._id, timeout) return self._refresh(podman) def remove(self, force=False): @@ -156,13 +156,13 @@ class Container(AttachMixin, StartMixin, collections.UserDict): force=True, stop running container. """ with self._client() as podman: - results = podman.RemoveContainer(self.id, force) + results = podman.RemoveContainer(self._id, force) return results['container'] def restart(self, timeout=25): """Restart container with timeout, return id on success.""" with self._client() as podman: - podman.RestartContainer(self.id, timeout) + podman.RestartContainer(self._id, timeout) return self._refresh(podman) def rename(self, target): @@ -182,13 +182,13 @@ class Container(AttachMixin, StartMixin, collections.UserDict): def pause(self): """Pause container, return id on success.""" with self._client() as podman: - podman.PauseContainer(self.id) + podman.PauseContainer(self._id) return self._refresh(podman) def unpause(self): """Unpause container, return id on success.""" with self._client() as podman: - podman.UnpauseContainer(self.id) + podman.UnpauseContainer(self._id) return self._refresh(podman) def update_container(self, *args, **kwargs): @@ -200,24 +200,24 @@ class Container(AttachMixin, StartMixin, collections.UserDict): def wait(self): """Wait for container to finish, return 'returncode'.""" with self._client() as podman: - results = podman.WaitContainer(self.id) + results = podman.WaitContainer(self._id) return int(results['exitcode']) def stats(self): """Retrieve resource stats from the container.""" with self._client() as podman: - results = podman.GetContainerStats(self.id) + results = podman.GetContainerStats(self._id) obj = results['container'] return collections.namedtuple('StatDetail', obj.keys())(**obj) def logs(self, *args, **kwargs): """Retrieve container logs.""" with self._client() as podman: - results = podman.GetContainerLogs(self.id) + results = podman.GetContainerLogs(self._id) yield from results -class Containers(object): +class Containers(): """Model for Containers collection.""" def __init__(self, client): @@ -237,9 +237,9 @@ class Containers(object): results = podman.DeleteStoppedContainers() return results['containers'] - def get(self, id): + def get(self, id_): """Retrieve container details from store.""" with self._client() as podman: - cntr = podman.GetContainer(id) + cntr = podman.GetContainer(id_) return Container(self._client, cntr['container']['id'], cntr['container']) diff --git a/contrib/python/podman/podman/libs/errors.py b/contrib/python/podman/podman/libs/errors.py index 3d2300e39..d34577a57 100644 --- a/contrib/python/podman/podman/libs/errors.py +++ b/contrib/python/podman/podman/libs/errors.py @@ -49,7 +49,7 @@ class PodmanError(VarlinkErrorProxy): pass -error_map = { +ERROR_MAP = { 'io.projectatomic.podman.ContainerNotFound': ContainerNotFound, 'io.projectatomic.podman.ErrorOccurred': ErrorOccurred, 'io.projectatomic.podman.ImageNotFound': ImageNotFound, @@ -60,6 +60,6 @@ error_map = { def error_factory(exception): """Map Exceptions to a discrete type.""" try: - return error_map[exception.error()](exception) + return ERROR_MAP[exception.error()](exception) except KeyError: return exception diff --git a/contrib/python/podman/podman/libs/images.py b/contrib/python/podman/podman/libs/images.py index 334ff873c..547994798 100644 --- a/contrib/python/podman/podman/libs/images.py +++ b/contrib/python/podman/podman/libs/images.py @@ -21,9 +21,9 @@ class Image(collections.UserDict): self._id = id self._client = client - assert self._id == self.id,\ + assert self._id == data['id'],\ 'Requested image id({}) does not match store id({})'.format( - self._id, self.id + self._id, data['id'] ) def __getitem__(self, key): @@ -40,7 +40,7 @@ class Image(collections.UserDict): """ details = self.inspect() - config = Config(image_id=self.id, **kwargs) + config = Config(image_id=self._id, **kwargs) config['command'] = details.containerconfig['cmd'] config['env'] = self._split_token(details.containerconfig['env']) config['image'] = copy.deepcopy(details.repotags[0]) @@ -48,45 +48,45 @@ class Image(collections.UserDict): config['net_mode'] = 'bridge' config['network'] = 'bridge' - logging.debug('Image {}: create config: {}'.format(self.id, config)) + logging.debug('Image %s: create config: %s', self._id, config) with self._client() as podman: - id = podman.CreateContainer(config)['container'] - cntr = podman.GetContainer(id) - return Container(self._client, id, cntr['container']) + id_ = podman.CreateContainer(config)['container'] + cntr = podman.GetContainer(id_) + return Container(self._client, id_, cntr['container']) container = create def export(self, dest, compressed=False): """Write image to dest, return id on success.""" with self._client() as podman: - results = podman.ExportImage(self.id, dest, compressed) + results = podman.ExportImage(self._id, dest, compressed) return results['image'] def history(self): """Retrieve image history.""" with self._client() as podman: - for r in podman.HistoryImage(self.id)['history']: + for r in podman.HistoryImage(self._id)['history']: yield collections.namedtuple('HistoryDetail', r.keys())(**r) # Convert all keys to lowercase. def _lower_hook(self): @functools.wraps(self._lower_hook) - def wrapped(input): - return {k.lower(): v for (k, v) in input.items()} + def wrapped(input_): + return {k.lower(): v for (k, v) in input_.items()} return wrapped def inspect(self): """Retrieve details about image.""" with self._client() as podman: - results = podman.InspectImage(self.id) + results = podman.InspectImage(self._id) obj = json.loads(results['image'], object_hook=self._lower_hook()) return collections.namedtuple('ImageInspect', obj.keys())(**obj) def push(self, target, tlsverify=False): """Copy image to target, return id on success.""" with self._client() as podman: - results = podman.PushImage(self.id, target, tlsverify) + results = podman.PushImage(self._id, target, tlsverify) return results['image'] def remove(self, force=False): @@ -95,17 +95,17 @@ class Image(collections.UserDict): force=True, stop any running containers using image. """ with self._client() as podman: - results = podman.RemoveImage(self.id, force) + results = podman.RemoveImage(self._id, force) return results['image'] def tag(self, tag): """Tag image.""" with self._client() as podman: - results = podman.TagImage(self.id, tag) + results = podman.TagImage(self._id, tag) return results['image'] -class Images(object): +class Images(): """Model for Images collection.""" def __init__(self, client): @@ -158,15 +158,15 @@ class Images(object): results = podman.PullImage(source) return results['id'] - def search(self, id, limit=25): + def search(self, id_, limit=25): """Search registries for id.""" with self._client() as podman: - results = podman.SearchImage(id, limit) + results = podman.SearchImage(id_, limit) for img in results['images']: yield collections.namedtuple('ImageSearch', img.keys())(**img) - def get(self, id): + def get(self, id_): """Get Image from id.""" with self._client() as podman: - result = podman.GetImage(id) + result = podman.GetImage(id_) return Image(self._client, result['image']['id'], result['image']) diff --git a/contrib/python/podman/podman/libs/system.py b/contrib/python/podman/podman/libs/system.py index c59867760..c611341e4 100644 --- a/contrib/python/podman/podman/libs/system.py +++ b/contrib/python/podman/podman/libs/system.py @@ -6,7 +6,7 @@ import pkg_resources from . import cached_property -class System(object): +class System(): """Model for accessing system resources.""" def __init__(self, client): @@ -22,7 +22,7 @@ class System(object): client = '0.0.0' try: client = pkg_resources.get_distribution('podman').version - except Exception: + except Exception: # pylint: disable=broad-except pass vers['client_version'] = client return collections.namedtuple('Version', vers.keys())(**vers) @@ -37,4 +37,4 @@ class System(object): """Return True if server awake.""" with self._client() as podman: response = podman.Ping() - return 'OK' == response['ping']['message'] + return response['ping']['message'] == 'OK' diff --git a/contrib/python/podman/podman/libs/tunnel.py b/contrib/python/podman/podman/libs/tunnel.py index 440eb3951..af02e690f 100644 --- a/contrib/python/podman/podman/libs/tunnel.py +++ b/contrib/python/podman/podman/libs/tunnel.py @@ -87,7 +87,7 @@ class Portal(collections.MutableMapping): self._schedule_reaper() -class Tunnel(object): +class Tunnel(): """SSH tunnel.""" def __init__(self, context): @@ -95,7 +95,7 @@ class Tunnel(object): self.context = context self._tunnel = None - def bore(self, id): + def bore(self, ident): """Create SSH tunnel from given context.""" cmd = ['ssh'] @@ -114,10 +114,10 @@ class Tunnel(object): cmd.append('ssh://{}@{}'.format(self.context.username, self.context.hostname)) - logging.debug('Tunnel cmd "{}"'.format(' '.join(cmd))) + logging.debug('Tunnel cmd "%s"', ' '.join(cmd)) self._tunnel = subprocess.Popen(cmd, close_fds=True) - for i in range(300): + for _ in range(300): # TODO: Make timeout configurable if os.path.exists(self.context.local_socket): break @@ -125,10 +125,10 @@ class Tunnel(object): else: raise TimeoutError('Failed to create tunnel using: {}'.format( ' '.join(cmd))) - weakref.finalize(self, self.close, id) + weakref.finalize(self, self.close, ident) return self - def close(self, id): + def close(self, ident): """Close SSH tunnel.""" if self._tunnel is None: return diff --git a/contrib/python/podman/test/test_images.py b/contrib/python/podman/test/test_images.py index 14bf90992..854f57dd7 100644 --- a/contrib/python/podman/test/test_images.py +++ b/contrib/python/podman/test/test_images.py @@ -109,7 +109,7 @@ class TestImages(PodmanTestCase): self.assertEqual(self.alpine_image.id, self.alpine_image.tag('alpine:fubar')) self.loadCache() - self.assertIn('alpine:fubar', self.alpine_image.repoTags) + self.assertIn('localhost/alpine:fubar', self.alpine_image.repoTags) def test_remove(self): before = self.loadCache() -- cgit v1.2.3-54-g00ecf