Skip to content

Commit

Permalink
Enable ruff specific rules (#2325)
Browse files Browse the repository at this point in the history
Warnings fixed:

* RUF001 String contains ambiguous `’` (RIGHT SINGLE QUOTATION MARK). Did you mean ``` (GRAVE ACCENT)?
* RUF002 Docstring contains ambiguous `’` (RIGHT SINGLE QUOTATION MARK). Did you mean ``` (GRAVE ACCENT)?
* RUF005 Consider iterable unpacking instead of concatenation
* RUF010 Use explicit conversion flag
* RUF013 PEP 484 prohibits implicit `Optional
* RUF015 Prefer `next(...)` over single element slice

Statistics:

22      RUF013  [*] implicit-optional
11      RUF005  [*] collection-literal-concatenation
 5      RUF002  [ ] ambiguous-unicode-character-docstring
 5      RUF015  [*] unnecessary-iterable-allocation-for-first-element
 3      RUF010  [*] explicit-f-string-type-conversion
 2      RUF001  [ ] ambiguous-unicode-character-string
  • Loading branch information
cutwater authored Oct 21, 2024
1 parent 73b9637 commit ce78a29
Show file tree
Hide file tree
Showing 23 changed files with 83 additions and 53 deletions.
8 changes: 6 additions & 2 deletions dev/common/dependency_bumper.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@
import shutil
import subprocess
import tempfile

from types import SimpleNamespace
from typing import Optional
from threading import Thread


Expand Down Expand Up @@ -121,7 +121,11 @@ def make_image() -> None:
shutil.rmtree(tdir)


def construct_checkout(checkout: str, base_branch: str = None, new_branch: str = None) -> None:
def construct_checkout(
checkout: str,
base_branch: Optional[str] = None,
new_branch: Optional[str] = None,
) -> None:
"""
construct_checkout makes a ready-to-go clone and branch of $REPO
Expand Down
2 changes: 1 addition & 1 deletion dev/oci_env_integration/actions/action_lib.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ def set_up_env(self):

def exec_cmd(self, env, cmd):
path = f"dev/oci_env_integration/oci_env_configs/{env}"
exec_cmd = ["oci-env", "-e", path] + shlex.split(cmd)
exec_cmd = ["oci-env", "-e", path, *shlex.split(cmd)]
print(" ".join(exec_cmd))
rc = subprocess.call(exec_cmd)

Expand Down
6 changes: 3 additions & 3 deletions galaxy_ng/app/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,11 +82,11 @@ def add_required_dab_attributes_to_models():
"reverse_name": "content-ansible/collection_signatures-detail"
},
pulp_ansible_models.CollectionVersion: {
"summary_fields": common_summary_fields + ("version",),
"summary_fields": (*common_summary_fields, "version"),
"reverse_name": "content-ansible/collection_versions-detail"
},
pulp_ansible_models.Collection: {
"summary_fields": common_summary_fields + ("namespace",),
"summary_fields": (*common_summary_fields, "namespace"),
"reverse_name": "ansible/collections-detail"
},
pulp_ansible_models.CollectionRemote: {
Expand All @@ -98,7 +98,7 @@ def add_required_dab_attributes_to_models():
"reverse_name": "repositories-ansible/ansible-detail"
},
galaxy_collectionimport_models.CollectionImport: {
"summary_fields": common_summary_fields + ("version",),
"summary_fields": (*common_summary_fields, "version"),
"reverse_name": "galaxy:api:v3:collection-imports-detail"
},
galaxy_namespace_models.NamespaceLink: {
Expand Down
3 changes: 2 additions & 1 deletion galaxy_ng/app/access_control/statements/standalone.py
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,8 @@
},
]

_group_statements = _group_role_statements + [
_group_statements = [
*_group_role_statements,
{
"action": ["create", "destroy", "update", "partial_update"],
"principal": "*",
Expand Down
3 changes: 1 addition & 2 deletions galaxy_ng/app/api/ui/v1/serializers/imports.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,5 +30,4 @@ class ImportTaskDetailSerializer(ImportTaskListSerializer):
messages = serializers.JSONField()

class Meta(ImportTaskListSerializer.Meta):
fields = ImportTaskListSerializer.Meta.fields + \
('error', 'messages')
fields = (*ImportTaskListSerializer.Meta.fields, 'error', 'messages')
2 changes: 1 addition & 1 deletion galaxy_ng/app/api/ui/v1/serializers/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ class CurrentUserSerializer(UserSerializer):

class Meta(UserSerializer.Meta):
model = auth_models.User
fields = UserSerializer.Meta.fields + ('model_permissions', 'is_anonymous',)
fields = (*UserSerializer.Meta.fields, 'model_permissions', 'is_anonymous')
extra_kwargs = dict(
groups={'read_only': True},
**UserSerializer.Meta.extra_kwargs
Expand Down
4 changes: 2 additions & 2 deletions galaxy_ng/app/api/v1/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ def do_git_checkout(clone_url, checkout_path, github_reference):

# REVIEW(cutwater): Why this iterates over all commits to get first one and doesn't use
# gitrepo.head.commit as the code below?
last_commit = list(gitrepo.iter_commits())[0]
last_commit = next(iter(gitrepo.iter_commits()))

else:
# use the default branch ...
Expand Down Expand Up @@ -428,7 +428,7 @@ def legacy_role_import(
result = import_legacy_role(checkout_path, namespace.name, importer_config, logger)
except Exception as e:
logger.info('')
logger.error(f'Role loading failed! {str(e)}')
logger.error(f'Role loading failed! {e}')
logger.info('')
tb_str = traceback.format_exc()
logger.error(tb_str)
Expand Down
7 changes: 4 additions & 3 deletions galaxy_ng/app/api/v3/viewsets/execution_environments.py
Original file line number Diff line number Diff line change
Expand Up @@ -307,10 +307,10 @@ def destroy(self, request, *args, **kwargs):
- Remove all tags that point to the selected manifest from the latest version of the repo.
- Remove the selected image manifest from the selected repository using the pulp container
remove_image function: This function will remove the manifest from the latest version
of the repository and any blobs associated with the manifest that arent used by
of the repository and any blobs associated with the manifest that aren't used by
other manifests.
- Call the reclaim disk space function on the selected repository, with the latest version
of the repository preserved. This will clear out artifacts for content that isnt in the
of the repository preserved. This will clear out artifacts for content that isn't in the
latest version of the repository.
"""
# Looks up the image via a sha
Expand All @@ -324,7 +324,8 @@ def destroy(self, request, *args, **kwargs):
).values_list("pk", flat=True)

# Remove the selected image manifest from the selected repository using the pulp container
content_unit_pks = [str(pk) for pk in list(tags_pks) + [manifest.pk]]
content_unit_pks = [str(pk) for pk in tags_pks]
content_unit_pks.append(str(manifest.pk))

# Call the recursive_remove_content from pulp_container + reclaim disk space
async_result = dispatch(
Expand Down
4 changes: 2 additions & 2 deletions galaxy_ng/app/dynaconf_hooks.py
Original file line number Diff line number Diff line change
Expand Up @@ -654,7 +654,7 @@ def validate(settings: Dynaconf) -> None:
settings.validators.register(
Validator(
"AUTHENTICATION_BACKEND_PRESET",
is_in=["local", "custom"] + list(presets.keys()),
is_in=["local", "custom", *presets.keys()],
)
)

Expand Down Expand Up @@ -788,7 +788,7 @@ def alter_hostname_settings(

def configure_dab_required_settings(settings: Dynaconf) -> Dict[str, Any]:
dab_settings = get_dab_settings(
installed_apps=settings.INSTALLED_APPS + ['ansible_base.jwt_consumer'],
installed_apps=[*settings.INSTALLED_APPS, 'ansible_base.jwt_consumer'],
rest_framework=settings.REST_FRAMEWORK,
spectacular_settings=settings.SPECTACULAR_SETTINGS,
authentication_backends=settings.AUTHENTICATION_BACKENDS,
Expand Down
2 changes: 1 addition & 1 deletion galaxy_ng/app/management/commands/dump-auth-config.py
Original file line number Diff line number Diff line change
Expand Up @@ -136,5 +136,5 @@ def handle(self, *args, **options):
self.stdout.write(json.dumps(data))

except Exception as e:
self.stdout.write(self.style.ERROR(f"An error occurred: {str(e)}"))
self.stdout.write(self.style.ERROR(f"An error occurred: {e}"))
sys.exit(1)
4 changes: 2 additions & 2 deletions galaxy_ng/app/migrations/_dab_rbac.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@

def pulp_role_to_single_content_type_or_none(pulprole):
content_types = {perm.content_type for perm in pulprole.permissions.all()}
if len(list(content_types)) == 1:
return list(content_types)[0]
if len(content_types) == 1:
return next(iter(content_types))
return None


Expand Down
6 changes: 3 additions & 3 deletions galaxy_ng/app/signals/__init__.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
"""
Strictly speaking, signal handling and registration code can live anywhere you like, although its
recommended to avoid the applications root module and its models module to minimize side-effects
Strictly speaking, signal handling and registration code can live anywhere you like, although it's
recommended to avoid the application's root module and its models module to minimize side-effects
of importing code.
In practice, signal handlers are usually defined in a signals submodule of the application they
relate to. Signal receivers are connected in the ready() method of your application configuration
class. If youre using the receiver() decorator, simply import the signals submodule inside
class. If you're using the receiver() decorator, simply import the signals submodule inside
ready().
https://stackoverflow.com/a/22924754
https://docs.djangoproject.com/en/3.2/topics/signals/
Expand Down
6 changes: 3 additions & 3 deletions galaxy_ng/app/signals/handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -172,8 +172,8 @@ def rbac_signal_in_progress():

def pulp_role_to_single_content_type_or_none(pulprole):
content_types = {perm.content_type for perm in pulprole.permissions.all()}
if len(list(content_types)) == 1:
return list(content_types)[0]
if len(content_types) == 1:
return next(iter(content_types))
return None


Expand Down Expand Up @@ -352,7 +352,7 @@ def lazy_content_type_correction(rd, obj):
except ValidationError as exc:
logger.warning(
f'Assignment to {rd.name} for {type(obj)}'
+ f' violates a DAB role validation rule: {str(exc)}'
+ f' violates a DAB role validation rule: {exc}'
)
return
rd.content_type = ct
Expand Down
6 changes: 3 additions & 3 deletions galaxy_ng/app/utils/legacy.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,11 @@


def sanitize_avatar_url(url):
'''Remove all the non-url characters people have put in their avatar urls'''
"""Remove all the non-url characters people have put in their avatar urls."""
regex = (
r"(?i)\b((?:https?://|www\d{0,3}[.]|[a-z0-9.\-]+[.][a-z]{2,4}/)"
+ r"(?:[^\s()<>]+|\(([^\s()<>]+|(\([^\s()<>]+\)))*\))+(?:\(([^\s()<>]+|"
+ r"(\([^\s()<>]+\)))*\)|[^\s`!()\[\]{};:'\".,<>?«»“”‘’]))"
+ r"(\([^\s()<>]+\)))*\)|[^\s`!()\[\]{};:'\".,<>?«»“”‘’]))" # noqa: RUF001
)

for match in re.findall(regex, url):
Expand All @@ -32,7 +32,7 @@ def sanitize_avatar_url(url):


def process_namespace(namespace_name, namespace_info, force=False):
'''Do all the work to sync a legacy namespace and build it's v3 counterpart'''
"""Do all the work to sync a legacy namespace and build it's v3 counterpart."""

logger.info(f'process legacy namespace ({namespace_info["id"]}) {namespace_name}')

Expand Down
2 changes: 1 addition & 1 deletion galaxy_ng/tests/integration/api/test_certified_sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ def _assert_sync(manifest, client):
for cv in manifest:
ns = cv["namespace"]
collection = (ns, cv["name"])
version = collection + (cv["version"], )
version = (*collection, cv["version"])

namespaces.add(ns)
versions.add(version)
Expand Down
2 changes: 1 addition & 1 deletion galaxy_ng/tests/integration/api/test_openapi.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ def test_openapi_bindings_generation(ansible_config, galaxy_client):

pulp_spec = gc.get('pulp/api/v3/docs/api.json')
status = gc.get('pulp/api/v3/status/')
version = [x['version'] for x in status['versions'] if x['component'] == 'galaxy'][0]
version = next(x['version'] for x in status['versions'] if x['component'] == 'galaxy')
my_id = subprocess.run('id -u', shell=True, capture_output=True).stdout.decode('utf-8').strip()
volume_name = '/local'
generator_repo = 'https://github.com/pulp/pulp-openapi-generator'
Expand Down
7 changes: 4 additions & 3 deletions galaxy_ng/tests/integration/utils/client_ansible_lib.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import json
import logging
from typing import Optional

import requests

Expand Down Expand Up @@ -178,11 +179,11 @@ def get_bearer_token(self, grant_type='password'):

def request(
self,
url: str = None,
url: Optional[str] = None,
args=None,
headers: dict = None,
headers: Optional[dict] = None,
method: str = 'GET',
auth_required: bool = None,
auth_required: Optional[bool] = None,
) -> dict:

"""
Expand Down
32 changes: 18 additions & 14 deletions galaxy_ng/tests/integration/utils/client_social_github.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import requests

from typing import Optional
from urllib.parse import urlparse

import requests

class SocialGithubClient:

""" An HTTP client to mimic github social auth"""
class SocialGithubClient:
""" An HTTP client to mimic GitHub social auth"""

_rs = None
_github_cookies = None
Expand Down Expand Up @@ -138,7 +138,11 @@ def logout(self, expected_code=None):
rr = self.post(absolute_url='/api/_ui/v1/auth/logout/', data={})
assert rr.status_code == 204, rr.text

def get(self, relative_url: str = None, absolute_url: str = None) -> requests.models.Response:
def get(
self,
relative_url: Optional[str] = None,
absolute_url: Optional[str] = None
) -> requests.models.Response:

pheaders = {
'Accept': 'application/json',
Expand All @@ -159,8 +163,8 @@ def get(self, relative_url: str = None, absolute_url: str = None) -> requests.mo

def delete(
self,
relative_url: str = None,
absolute_url: str = None
relative_url: Optional[str] = None,
absolute_url: Optional[str] = None,
) -> requests.models.Response:

pheaders = {
Expand All @@ -182,9 +186,9 @@ def delete(

def put(
self,
relative_url: str = None,
absolute_url: str = None,
data=None
relative_url: Optional[str] = None,
absolute_url: Optional[str] = None,
data=None,
) -> requests.models.Response:

pheaders = {
Expand All @@ -206,8 +210,8 @@ def put(

def patch(
self,
relative_url: str = None,
absolute_url: str = None,
relative_url: Optional[str] = None,
absolute_url: Optional[str] = None,
data=None
) -> requests.models.Response:

Expand All @@ -230,8 +234,8 @@ def patch(

def post(
self,
relative_url: str = None,
absolute_url: str = None,
relative_url: Optional[str] = None,
absolute_url: Optional[str] = None,
data=None
) -> requests.models.Response:

Expand Down
14 changes: 12 additions & 2 deletions galaxy_ng/tests/integration/utils/client_ui.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
"""Utility functions for AH tests."""

import logging
from typing import Optional

import requests
import re
import html
Expand Down Expand Up @@ -145,7 +147,11 @@ def logout(self, expected_code=None):
if res.status_code != expected_code:
raise Exception(f'logout status code was not {expected_code}')

def get(self, relative_url: str = None, absolute_url: str = None) -> requests.models.Response:
def get(
self,
relative_url: Optional[str] = None,
absolute_url: Optional[str] = None
) -> requests.models.Response:

pheaders = {
'Accept': 'application/json',
Expand All @@ -172,7 +178,11 @@ def get(self, relative_url: str = None, absolute_url: str = None) -> requests.mo
raise_for_status(resp)
return resp

def get_paginated(self, relative_url: str = None, absolute_url: str = None) -> list:
def get_paginated(
self,
relative_url: Optional[str] = None,
absolute_url: Optional[str] = None
) -> list:
"""Iterate through all results in a paginated queryset"""
if absolute_url:
resp = self.get(absolute_url=absolute_url)
Expand Down
2 changes: 1 addition & 1 deletion galaxy_ng/tests/integration/utils/collections.py
Original file line number Diff line number Diff line change
Expand Up @@ -447,7 +447,7 @@ def set_certification(config, gc, collection, level="published", hub_4_5=False):
repository_pulp_href = gc.get(rep_obj_url)["results"][0]["pulp_href"]
artifact_obj_url = f"_ui/v1/repo/staging/{collection.namespace}/" f"{collection.name}/"
all_versions = gc.get(artifact_obj_url)["all_versions"]
one_version = [v for v in all_versions if v["version"] == collection.version][0]
one_version = next(v for v in all_versions if v["version"] == collection.version)
artifact_pulp_id = one_version["id"]
artifact_pulp_href = (
"/"
Expand Down
3 changes: 2 additions & 1 deletion galaxy_ng/tests/integration/utils/iqe_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import time
from functools import lru_cache
from json import JSONDecodeError
from typing import Optional
from unittest.mock import patch

from pkg_resources import parse_version
Expand Down Expand Up @@ -757,7 +758,7 @@ def galaxy_auto_sign_collections():
return settings.get("GALAXY_AUTO_SIGN_COLLECTIONS")


def get_paginated(client, relative_url: str = None) -> list:
def get_paginated(client, relative_url: Optional[str] = None) -> list:
"""Iterate through all results in a paginated queryset"""
ds = client.get(relative_url)

Expand Down
Loading

0 comments on commit ce78a29

Please sign in to comment.