Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switch linting to ruff, use f-strings, fix lint errors and possible bugs #3126

Merged
merged 13 commits into from
Aug 15, 2023
Merged
10 changes: 5 additions & 5 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,16 @@ env:
DOCKER_BUILDKIT: '1'

jobs:
flake8:
lint:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
- uses: actions/setup-python@v4
with:
python-version: '3.x'
- run: pip install -U flake8
- name: Run flake8
run: flake8 docker/ tests/
python-version: '3.11'
- run: pip install -U ruff==0.0.284
- name: Run ruff
run: ruff docker tests

unit-tests:
runs-on: ubuntu-latest
Expand Down
2 changes: 1 addition & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ paragraph in the Docker contribution guidelines.
Before we can review your pull request, please ensure that nothing has been
broken by your changes by running the test suite. You can do so simply by
running `make test` in the project root. This also includes coding style using
`flake8`
`ruff`

### 3. Write clear, self-contained commits

Expand Down
8 changes: 4 additions & 4 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ build-dind-certs:
docker build -t dpy-dind-certs -f tests/Dockerfile-dind-certs .

.PHONY: test
test: flake8 unit-test-py3 integration-dind integration-dind-ssl
test: ruff unit-test-py3 integration-dind integration-dind-ssl

.PHONY: unit-test-py3
unit-test-py3: build-py3
Expand Down Expand Up @@ -163,9 +163,9 @@ integration-dind-ssl: build-dind-certs build-py3 setup-network

docker rm -vf dpy-dind-ssl dpy-dind-certs

.PHONY: flake8
flake8: build-py3
docker run -t --rm docker-sdk-python3 flake8 docker tests
.PHONY: ruff
ruff: build-py3
docker run -t --rm docker-sdk-python3 ruff docker tests

.PHONY: docs
docs: build-docs
Expand Down
1 change: 0 additions & 1 deletion docker/__init__.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
# flake8: noqa
from .api import APIClient
from .client import DockerClient, from_env
from .context import Context
Expand Down
1 change: 0 additions & 1 deletion docker/api/__init__.py
Original file line number Diff line number Diff line change
@@ -1,2 +1 @@
# flake8: noqa
from .client import APIClient
14 changes: 5 additions & 9 deletions docker/api/build.py
Original file line number Diff line number Diff line change
Expand Up @@ -314,9 +314,8 @@ def _set_auth_headers(self, headers):
auth_data[auth.INDEX_URL] = auth_data.get(auth.INDEX_NAME, {})

log.debug(
'Sending auth config ({})'.format(
', '.join(repr(k) for k in auth_data.keys())
)
"Sending auth config (%s)",
', '.join(repr(k) for k in auth_data),
)

if auth_data:
Expand All @@ -336,12 +335,9 @@ def process_dockerfile(dockerfile, path):
abs_dockerfile = os.path.join(path, dockerfile)
if constants.IS_WINDOWS_PLATFORM and path.startswith(
constants.WINDOWS_LONGPATH_PREFIX):
abs_dockerfile = '{}{}'.format(
constants.WINDOWS_LONGPATH_PREFIX,
os.path.normpath(
abs_dockerfile[len(constants.WINDOWS_LONGPATH_PREFIX):]
)
)
normpath = os.path.normpath(
abs_dockerfile[len(constants.WINDOWS_LONGPATH_PREFIX):])
abs_dockerfile = f'{constants.WINDOWS_LONGPATH_PREFIX}{normpath}'
if (os.path.splitdrive(path)[0] != os.path.splitdrive(abs_dockerfile)[0] or
os.path.relpath(abs_dockerfile, path).startswith('..')):
# Dockerfile not in context - read data to insert into tar later
Expand Down
35 changes: 16 additions & 19 deletions docker/api/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -160,10 +160,10 @@ def __init__(self, base_url=None, version=None,
base_url, timeout, pool_connections=num_pools,
max_pool_size=max_pool_size
)
except NameError:
except NameError as err:
raise DockerException(
'Install pypiwin32 package to enable npipe:// support'
)
) from err
self.mount('http+docker://', self._custom_adapter)
self.base_url = 'http+docker://localnpipe'
elif base_url.startswith('ssh://'):
Expand All @@ -172,10 +172,10 @@ def __init__(self, base_url=None, version=None,
base_url, timeout, pool_connections=num_pools,
max_pool_size=max_pool_size, shell_out=use_ssh_client
)
except NameError:
except NameError as err:
raise DockerException(
'Install paramiko package to enable ssh:// support'
)
) from err
self.mount('http+docker://ssh', self._custom_adapter)
self._unmount('http://', 'https://')
self.base_url = 'http+docker://ssh'
Expand All @@ -199,28 +199,27 @@ def __init__(self, base_url=None, version=None,
self._version = version
if not isinstance(self._version, str):
raise DockerException(
'Version parameter must be a string or None. Found {}'.format(
type(version).__name__
)
'Version parameter must be a string or None. '
f'Found {type(version).__name__}'
)
if utils.version_lt(self._version, MINIMUM_DOCKER_API_VERSION):
raise InvalidVersion(
'API versions below {} are no longer supported by this '
'library.'.format(MINIMUM_DOCKER_API_VERSION)
f'API versions below {MINIMUM_DOCKER_API_VERSION} are '
f'no longer supported by this library.'
)

def _retrieve_server_version(self):
try:
return self.version(api_version=False)["ApiVersion"]
except KeyError:
except KeyError as ke:
raise DockerException(
'Invalid response from docker daemon: key "ApiVersion"'
' is missing.'
)
) from ke
except Exception as e:
raise DockerException(
f'Error while fetching server API version: {e}'
)
) from e

def _set_request_timeout(self, kwargs):
"""Prepare the kwargs for an HTTP request by inserting the timeout
Expand Down Expand Up @@ -248,19 +247,17 @@ def _url(self, pathfmt, *args, **kwargs):
for arg in args:
if not isinstance(arg, str):
raise ValueError(
'Expected a string but found {} ({}) '
'instead'.format(arg, type(arg))
f'Expected a string but found {arg} ({type(arg)}) instead'
)

quote_f = partial(urllib.parse.quote, safe="/:")
args = map(quote_f, args)

formatted_path = pathfmt.format(*args)
if kwargs.get('versioned_api', True):
return '{}/v{}{}'.format(
self.base_url, self._version, pathfmt.format(*args)
)
return f'{self.base_url}/v{self._version}{formatted_path}'
else:
return f'{self.base_url}{pathfmt.format(*args)}'
return f'{self.base_url}{formatted_path}'

def _raise_for_status(self, response):
"""Raises stored :class:`APIError`, if one occurred."""
Expand Down Expand Up @@ -479,7 +476,7 @@ def _get_result_tty(self, stream, res, is_tty):
return self._multiplexed_response_stream_helper(res)
else:
return sep.join(
[x for x in self._multiplexed_buffer_helper(res)]
list(self._multiplexed_buffer_helper(res))
)

def _unmount(self, *args):
Expand Down
10 changes: 5 additions & 5 deletions docker/api/container.py
Original file line number Diff line number Diff line change
Expand Up @@ -863,8 +863,8 @@ def logs(self, container, stdout=True, stderr=True, stream=False,
params['since'] = since
else:
raise errors.InvalidArgument(
'since value should be datetime or positive int/float, '
'not {}'.format(type(since))
'since value should be datetime or positive int/float,'
f' not {type(since)}'
)

if until is not None:
Expand All @@ -880,8 +880,8 @@ def logs(self, container, stdout=True, stderr=True, stream=False,
params['until'] = until
else:
raise errors.InvalidArgument(
'until value should be datetime or positive int/float, '
'not {}'.format(type(until))
f'until value should be datetime or positive int/float, '
f'not {type(until)}'
)

url = self._url("/containers/{0}/logs", container)
Expand Down Expand Up @@ -953,7 +953,7 @@ def port(self, container, private_port):
return port_settings.get(private_port)

for protocol in ['tcp', 'udp', 'sctp']:
h_ports = port_settings.get(private_port + '/' + protocol)
h_ports = port_settings.get(f"{private_port}/{protocol}")
if h_ports:
break

Expand Down
4 changes: 1 addition & 3 deletions docker/api/service.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,7 @@ def _check_api_features(version, task_template, update_config, endpoint_spec,

def raise_version_error(param, min_version):
raise errors.InvalidVersion(
'{} is not supported in API version < {}'.format(
param, min_version
)
f'{param} is not supported in API version < {min_version}'
)

if update_config is not None:
Expand Down
29 changes: 10 additions & 19 deletions docker/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,15 @@ def resolve_repository_name(repo_name):
index_name, remote_name = split_repo_name(repo_name)
if index_name[0] == '-' or index_name[-1] == '-':
raise errors.InvalidRepository(
'Invalid index name ({}). Cannot begin or end with a'
' hyphen.'.format(index_name)
f'Invalid index name ({index_name}). '
'Cannot begin or end with a hyphen.'
)
return resolve_index_name(index_name), remote_name


def resolve_index_name(index_name):
index_name = convert_to_hostname(index_name)
if index_name == 'index.' + INDEX_NAME:
if index_name == f"index.{INDEX_NAME}":
index_name = INDEX_NAME
return index_name

Expand Down Expand Up @@ -99,27 +99,19 @@ def parse_auth(cls, entries, raise_on_error=False):
for registry, entry in entries.items():
if not isinstance(entry, dict):
log.debug(
'Config entry for key {} is not auth config'.format(
registry
)
f'Config entry for key {registry} is not auth config'
)
# We sometimes fall back to parsing the whole config as if it
# was the auth config by itself, for legacy purposes. In that
# case, we fail silently and return an empty conf if any of the
# keys is not formatted properly.
if raise_on_error:
raise errors.InvalidConfigFile(
'Invalid configuration for registry {}'.format(
registry
)
f'Invalid configuration for registry {registry}'
)
return {}
if 'identitytoken' in entry:
log.debug(
'Found an IdentityToken entry for registry {}'.format(
registry
)
)
log.debug(f'Found an IdentityToken entry for registry {registry}')
conf[registry] = {
'IdentityToken': entry['identitytoken']
}
Expand All @@ -130,16 +122,15 @@ def parse_auth(cls, entries, raise_on_error=False):
# a valid value in the auths config.
# https://github.com/docker/compose/issues/3265
log.debug(
'Auth data for {} is absent. Client might be using a '
'credentials store instead.'.format(registry)
f'Auth data for {registry} is absent. '
f'Client might be using a credentials store instead.'
)
conf[registry] = {}
continue

username, password = decode_auth(entry['auth'])
log.debug(
'Found entry (registry={}, username={})'
.format(repr(registry), repr(username))
f'Found entry (registry={registry!r}, username={username!r})'
)

conf[registry] = {
Expand Down Expand Up @@ -277,7 +268,7 @@ def _resolve_authconfig_credstore(self, registry, credstore_name):
except credentials.StoreError as e:
raise errors.DockerException(
f'Credentials store error: {repr(e)}'
)
) from e

def _get_store_instance(self, name):
if name not in self._stores:
Expand Down
1 change: 0 additions & 1 deletion docker/context/__init__.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,2 @@
# flake8: noqa
from .context import Context
from .api import ContextAPI
4 changes: 2 additions & 2 deletions docker/context/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,8 @@ def contexts(cls):
names.append(data["Name"])
except Exception as e:
raise errors.ContextException(
"Failed to load metafile {}: {}".format(
filename, e))
f"Failed to load metafile {filename}: {e}",
) from e

contexts = [cls.DEFAULT_CONTEXT]
for name in names:
Expand Down
3 changes: 2 additions & 1 deletion docker/context/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,5 +77,6 @@ def get_context_host(path=None, tls=False):
host = utils.parse_host(path, IS_WINDOWS_PLATFORM, tls)
if host == DEFAULT_UNIX_SOCKET:
# remove http+ from default docker socket url
return host.strip("http+")
if host.startswith("http+"):
host = host[5:]
return host
10 changes: 6 additions & 4 deletions docker/context/context.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,9 @@ def __init__(self, name, orchestrator=None, host=None, endpoints=None,
for k, v in endpoints.items():
if not isinstance(v, dict):
# unknown format
raise ContextException("""Unknown endpoint format for
context {}: {}""".format(name, v))
raise ContextException(
f"Unknown endpoint format for context {name}: {v}",
)

self.endpoints[k] = v
if k != "docker":
Expand Down Expand Up @@ -96,8 +97,9 @@ def _load_meta(cls, name):
metadata = json.load(f)
except (OSError, KeyError, ValueError) as e:
# unknown format
raise Exception("""Detected corrupted meta file for
context {} : {}""".format(name, e))
raise Exception(
f"Detected corrupted meta file for context {name} : {e}"
) from e

# for docker endpoints, set defaults for
# Host and SkipTLSVerify fields
Expand Down
8 changes: 6 additions & 2 deletions docker/credentials/__init__.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
# flake8: noqa
from .store import Store
from .errors import StoreError, CredentialsNotFound
from .constants import *
from .constants import (
DEFAULT_LINUX_STORE,
DEFAULT_OSX_STORE,
DEFAULT_WIN32_STORE,
PROGRAM_PREFIX,
)
12 changes: 2 additions & 10 deletions docker/credentials/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,5 @@ class InitializationError(StoreError):
def process_store_error(cpe, program):
message = cpe.output.decode('utf-8')
if 'credentials not found in native keychain' in message:
return CredentialsNotFound(
'No matching credentials in {}'.format(
program
)
)
return StoreError(
'Credentials store {} exited with "{}".'.format(
program, cpe.output.decode('utf-8').strip()
)
)
return CredentialsNotFound(f'No matching credentials in {program}')
return StoreError(f'Credentials store {program} exited with "{message}".')