diff --git a/pyproject.toml b/pyproject.toml index e68f3001..0304d4b7 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -43,11 +43,8 @@ dev = [ "tox-uv", "aiohttp>=3", "asyncio_extras", - "black", "coveralls", - "flake8", - "flake8_docstrings", - "isort>=5", + "ruff", "mock", "pytest", "pytest-asyncio", @@ -85,34 +82,24 @@ verify_cot = "scriptworker.cot.verify:verify_cot_cmdln" create_test_workdir = "scriptworker.cot.verify:create_test_workdir" verify_ed25519_signature = "scriptworker.ed25519:verify_ed25519_signature_cmdln" -[tool.black] +[tool.ruff] line-length = 160 -target-version = ["py311", "py312", "py313"] -include = '\.(wsgi|pyi?)$' -exclude = ''' -/( - \.eggs - | \.git - | \.hg - | \.mypy_cache - | \.tox - | \.venv - | \.cache - | \.cache_py3 - | _build - | buck-out - | build - | dist - | ui -)/ -''' +target-version = "py311" +extend-exclude = [ + "docs" +] + +[tool.ruff.lint] +# F = pyflakes, E/W = pycodestyle, I = isort, D = pydocstyle +select = ["F", "E", "W", "I", "D"] + +[tool.ruff.lint.per-file-ignores] +"tests/**" = ["D"] +"tests/test_github.py" = ["D", "E501"] # ignore long lines in test_github.py + -[tool.isort] -multi_line_output = 3 -include_trailing_comma = true -force_grid_wrap = 0 -use_parentheses = true -line_length = 160 +[tool.ruff.lint.pydocstyle] +convention = "pep257" [tool.pytest.ini_options] asyncio_mode = "auto" diff --git a/src/scriptworker/artifacts.py b/src/scriptworker/artifacts.py index 6ae2e23b..6741168d 100644 --- a/src/scriptworker/artifacts.py +++ b/src/scriptworker/artifacts.py @@ -225,6 +225,7 @@ def get_artifact_url(context, task_id, path): # list_latest_artifacts {{{1 async def list_latest_artifacts(queue, task_id): + """List all artifacts for the latest run of a task, following pagination.""" artifacts = [] def pagination_handler(response): @@ -235,6 +236,7 @@ def pagination_handler(response): async def retry_list_latest_artifacts(queue, task_id, exception=TaskclusterFailure, **kwargs): + """Retry ``list_latest_artifacts`` on transient Taskcluster failures.""" kwargs.setdefault("retry_exceptions", tuple(set([TaskclusterFailure, exception]))) return await retry_async(list_latest_artifacts, args=(queue, task_id), **kwargs) @@ -442,6 +444,7 @@ def assert_is_parent(path, parent_dir): def get_artifacts_matching_glob(context, task_id, pattern): + """Return paths of downloaded artifacts matching ``pattern`` under the task's cot/ dir.""" parent_dir = os.path.abspath(os.path.join(context.config["work_dir"], "cot", task_id)) matching = [] for root, _, files in os.walk(parent_dir): diff --git a/src/scriptworker/context.py b/src/scriptworker/context.py index dbaebffc..f60059e5 100644 --- a/src/scriptworker/context.py +++ b/src/scriptworker/context.py @@ -261,6 +261,7 @@ async def populate_projects(self, force: bool = False) -> None: @property def download_semaphore(self) -> asyncio.BoundedSemaphore: + """Return a lazily-built semaphore limiting concurrent downloads to ``max_concurrent_downloads``.""" assert self.config if self._download_semaphore is None: try: diff --git a/src/scriptworker/cot/verify.py b/src/scriptworker/cot/verify.py index 92a23954..292e3fe8 100644 --- a/src/scriptworker/cot/verify.py +++ b/src/scriptworker/cot/verify.py @@ -180,7 +180,8 @@ def is_decision(self): def is_scope_in_restricted_scopes(self, scope, restricted_scopes): """Determine if a scope matches in a list of restricted_scopes. - if one of the restricted scopes ends with '*', find a partial match. + + If one of the restricted scopes ends with '*', find a partial match. Returns: String: string of matching restricted_scope, if no match "" @@ -728,8 +729,10 @@ async def download_cot_artifact(chain, task_id, path): link = chain.get_link(task_id) log.debug("Verifying {} is in {} cot artifacts...".format(path, task_id)) if not link.cot: - log.warning('Chain of Trust for "{}" in {} does not exist. See above log for more details. \ -Skipping download of this artifact'.format(path, task_id)) + log.warning( + 'Chain of Trust for "{}" in {} does not exist. See above log for more details. \ +Skipping download of this artifact'.format(path, task_id) + ) return if path not in link.cot["artifacts"]: @@ -1094,8 +1097,9 @@ async def _get_additional_hg_push_jsone_context(parent_link, decision_link): allowed_comments.append(push_comment[try_idx:].split("\n")[0]) if decision_comment not in allowed_comments: raise CoTError( - "Decision task {} comment doesn't match the push comment!\n" - "Decision comment: \n{}\nPush comment: \n{}".format(decision_link.name, decision_comment, push_comment) + "Decision task {} comment doesn't match the push comment!\nDecision comment: \n{}\nPush comment: \n{}".format( + decision_link.name, decision_comment, push_comment + ) ) return { "push": { @@ -2113,7 +2117,8 @@ def verify_cot_cmdln(args=None, event_loop=None): """ args = args or sys.argv[1:] - parser = argparse.ArgumentParser(description="""Verify a given task's chain of trust. + parser = argparse.ArgumentParser( + description="""Verify a given task's chain of trust. Given a task's `task_id`, get its task definition, then trace its chain of trust back to the tree. This doesn't verify chain of trust artifact signatures, @@ -2125,7 +2130,8 @@ def verify_cot_cmdln(args=None, event_loop=None): or in the CREDS_FILES http://bit.ly/2fVMu0A If you are verifying against a private github repo, please also set in environment -SCRIPTWORKER_GITHUB_OAUTH_TOKEN to an OAUTH token with read permissions to the repo""") +SCRIPTWORKER_GITHUB_OAUTH_TOKEN to an OAUTH token with read permissions to the repo""" + ) parser.add_argument("task_id", help="the task id to test") parser.add_argument("--task-type", help="the task type to test", choices=sorted(get_valid_task_types().keys()), required=True) parser.add_argument("--cleanup", help="clean up the temp dir afterwards", dest="cleanup", action="store_true", default=False) @@ -2192,13 +2198,15 @@ def create_test_workdir(args=None, event_loop=None): """ args = args or sys.argv[1:] - parser = argparse.ArgumentParser(description="""Populate a test `work_dir`. + parser = argparse.ArgumentParser( + description="""Populate a test `work_dir`. Given a scriptworker task's `task_id`, get its task definition, write it to `./work/task.json`, then download its `upstreamArtifacts` and put them in `./work/cot/TASK_ID/PATH`. -This is helpful in manually testing a *script run.""") +This is helpful in manually testing a *script run.""" + ) parser.add_argument("--path", help="relative path to the work_dir", default="work") parser.add_argument("--overwrite", help="overwrite an existing work_dir", action="store_true") parser.add_argument("task_id", help="the task id to test") diff --git a/src/scriptworker/ed25519.py b/src/scriptworker/ed25519.py index caf67672..e5d1513c 100644 --- a/src/scriptworker/ed25519.py +++ b/src/scriptworker/ed25519.py @@ -141,11 +141,13 @@ def verify_ed25519_signature_cmdln(args=None, exception=SystemExit): """ args = args or sys.argv[1:] - parser = argparse.ArgumentParser(description="""Verify an ed25519 signature from the command line. + parser = argparse.ArgumentParser( + description="""Verify an ed25519 signature from the command line. Given a file and its detached signature, verify that it has been signed with a valid key. This key can be specified on the command line; otherwise we'll -default to ``config['ed25519_public_keys']``.""") +default to ``config['ed25519_public_keys']``.""" + ) parser.add_argument("--pubkey", help="path to a base64-encoded ed25519 pubkey, optional") parser.add_argument("file_path") parser.add_argument("sig_path") diff --git a/src/scriptworker/github.py b/src/scriptworker/github.py index 485c1286..0a7b2154 100644 --- a/src/scriptworker/github.py +++ b/src/scriptworker/github.py @@ -121,7 +121,7 @@ async def has_commit_landed_on_repository(self, context, revision): if any(vcs_rule.get("require_secret") for vcs_rule in context.config["trusted_vcs_rules"]): # This check uses unofficial API on github, which we can't easily # check for private repos, assume its true in the private case. - log.info("has_commit_landed_on_repository() not implemented for private" "repositories, assume True") + log.info("has_commit_landed_on_repository() not implemented for private repositories, assume True") return True # Revision may be a tag name. `branch_commits` doesn't work on tags @@ -293,7 +293,7 @@ def is_github_repo_owner_the_official_one(context, repo_owner): official_repo_owner = context.config["official_github_repos_owner"] if not official_repo_owner: raise ConfigError( - "This worker does not have a defined owner for official GitHub repositories. " 'Given "official_github_repos_owner": {}'.format(official_repo_owner) + 'This worker does not have a defined owner for official GitHub repositories. Given "official_github_repos_owner": {}'.format(official_repo_owner) ) return official_repo_owner == repo_owner diff --git a/src/scriptworker/task.py b/src/scriptworker/task.py index 48815e02..4f4751ab 100644 --- a/src/scriptworker/task.py +++ b/src/scriptworker/task.py @@ -117,7 +117,7 @@ def get_run_id(claim_task): # get_task_maxruntime {{{1 def get_task_maxruntime(task_def, max_timeout): - """Given a task definition, return the lower of max_timeout and maxRunTime if set""" + """Given a task definition, return the lower of max_timeout and maxRunTime if set.""" max_run_time = task_def["payload"].get("maxRunTime") if max_run_time is None: return max_timeout diff --git a/src/scriptworker/task_process.py b/src/scriptworker/task_process.py index 4f811f80..7c0a722a 100644 --- a/src/scriptworker/task_process.py +++ b/src/scriptworker/task_process.py @@ -19,7 +19,7 @@ class TaskProcess: """Wraps worker task's process.""" def __init__(self, process: Process): - """Constructor. + """Initialize the wrapper around the worker task's subprocess. Args: process (Process): task process diff --git a/src/scriptworker/utils.py b/src/scriptworker/utils.py index 5c231dc1..df21428f 100644 --- a/src/scriptworker/utils.py +++ b/src/scriptworker/utils.py @@ -42,6 +42,7 @@ # scriptworker_session {{{1 def scriptworker_session(*args, **kwargs): + """Return an ``aiohttp.ClientSession`` with the scriptworker User-Agent header set.""" kwargs.setdefault("headers", {}).setdefault("User-Agent", f"scriptworker {scriptworker.__version__}") return aiohttp.ClientSession(*args, **kwargs) @@ -875,8 +876,9 @@ def remove_empty_keys(values, remove=({}, None, [], "null")): # add_projectid {{{1 def add_projectid(task_def): - """Add a projectId property to a task, if none already exists, using - the Taskcluster default value of 'none'. + """Add a projectId property to a task, if none already exists. + + Uses the Taskcluster default value of 'none'. Args: task_def (dict): the task definition @@ -889,8 +891,9 @@ def add_projectid(task_def): # add_taskqueueid {{{1 def add_taskqueueid(task_def): - """Add a taskQueueId property to a task, if none already exists, based - on the provisionerId and workerType. Then remove those two properties. + """Add a taskQueueId property to a task, if none already exists. + + Builds the value from the provisionerId and workerType, then removes those two properties. Args: task_def (dict): the task definition diff --git a/src/scriptworker/worker.py b/src/scriptworker/worker.py index 3c04cd83..d07e0730 100644 --- a/src/scriptworker/worker.py +++ b/src/scriptworker/worker.py @@ -104,7 +104,7 @@ class RunTasks: """Manages processing of Taskcluster tasks.""" def __init__(self): - """Constructor.""" + """Initialize the task-processing state.""" self.future = None self.task_process = None self.is_cancelled = False diff --git a/taskcluster/scriptworker_taskgraph/__init__.py b/taskcluster/scriptworker_taskgraph/__init__.py index d55eb764..44162e15 100644 --- a/taskcluster/scriptworker_taskgraph/__init__.py +++ b/taskcluster/scriptworker_taskgraph/__init__.py @@ -1,15 +1,15 @@ # This Source Code Form is subject to the terms of the Mozilla Public # License, v. 2.0. If a copy of the MPL was not distributed with this # file, You can obtain one at http://mozilla.org/MPL/2.0/. - +"""Scriptworker taskgraph extensions.""" from importlib import import_module def register(graph_config): - """ - Import all modules that are siblings of this one, triggering decorators in - the process. + """Import all sibling modules to trigger their decorator-based registration. + + Imports each sibling module so its top-level decorators (e.g. transforms) run. """ _import_modules( [ diff --git a/taskcluster/scriptworker_taskgraph/transforms/__init__.py b/taskcluster/scriptworker_taskgraph/transforms/__init__.py index e69de29b..5fe370e1 100644 --- a/taskcluster/scriptworker_taskgraph/transforms/__init__.py +++ b/taskcluster/scriptworker_taskgraph/transforms/__init__.py @@ -0,0 +1 @@ +"""Scriptworker-specific taskgraph transforms.""" diff --git a/taskcluster/scriptworker_taskgraph/transforms/tox.py b/taskcluster/scriptworker_taskgraph/transforms/tox.py index 06d3929e..f9f45912 100644 --- a/taskcluster/scriptworker_taskgraph/transforms/tox.py +++ b/taskcluster/scriptworker_taskgraph/transforms/tox.py @@ -1,9 +1,7 @@ # This Source Code Form is subject to the terms of the Mozilla Public # License, v. 2.0. If a copy of the MPL was not distributed with this # file, You can obtain one at http://mozilla.org/MPL/2.0/. -""" -Tox-specific transforms -""" +"""Tox-specific transforms.""" from copy import deepcopy @@ -43,6 +41,7 @@ def _resolve_replace_string(item, field, subs): @transforms.add def replace_strings(config, jobs): + """Substitute python-version / targets / name placeholders in selected job fields.""" fields = [ "description", "run.command", @@ -65,6 +64,7 @@ def replace_strings(config, jobs): @transforms.add def update_env(config, jobs): + """Merge each job's ``env`` mapping into its worker ``env`` and drop the top-level key.""" for job in jobs: env = job.pop("env", {}) job["worker"].setdefault("env", {}).update(env) diff --git a/tests/test_cot_verify.py b/tests/test_cot_verify.py index 043f8af9..7a73354a 100644 --- a/tests/test_cot_verify.py +++ b/tests/test_cot_verify.py @@ -1405,7 +1405,7 @@ def test_get_action_perm(defn, expected): @pytest.mark.asyncio @pytest.mark.parametrize( - "name,task_id,path,decision_task_id,decision_path," "expected_template_path,expected_context_path", + "name,task_id,path,decision_task_id,decision_path,expected_template_path,expected_context_path", ( ( "action", diff --git a/tests/test_github.py b/tests/test_github.py index 5a51677e..4d315f76 100644 --- a/tests/test_github.py +++ b/tests/test_github.py @@ -6,7 +6,7 @@ import pytest from scriptworker import github -from scriptworker.exceptions import ConfigError +from scriptworker.exceptions import ConfigError, ScriptWorkerRetryException @pytest.fixture(autouse=True) @@ -220,7 +220,7 @@ async def test_has_commit_landed_on_repository_private(vpn_context, github_repos async def retry_request(_, url): assert False, "We should never have made a request" - assert await github_repository.has_commit_landed_on_repository(vpn_context, commitish) == True + assert await github_repository.has_commit_landed_on_repository(vpn_context, commitish) @pytest.mark.asyncio diff --git a/tests/test_integration.py b/tests/test_integration.py index a263abca..97321ef9 100644 --- a/tests/test_integration.py +++ b/tests/test_integration.py @@ -37,7 +37,8 @@ def read_integration_creds(): creds = read_worker_creds(key="integration_credentials") if creds: return creds - raise Exception("""To run integration tests, put your worker-test clientId creds, in json format, + raise Exception( + """To run integration tests, put your worker-test clientId creds, in json format, in one of these files: {files} @@ -54,7 +55,8 @@ def read_integration_creds(): This clientId will need the scope assume:project:taskcluster:worker-test-scopes -To skip integration tests, set the environment variable NO_CREDENTIALS_TESTS""".format(files=CREDS_FILES)) +To skip integration tests, set the environment variable NO_CREDENTIALS_TESTS""".format(files=CREDS_FILES) + ) def build_config(override, basedir): diff --git a/tests/test_utils.py b/tests/test_utils.py index 79c30196..1f222498 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -25,7 +25,7 @@ # some basic utf-8 test strings - these should all pass "english": "The quick brown fox jumped over the lazy dog", # this hiragana pangram comes from http://www.columbia.edu/~fdc/utf8/ - "hiragana": "いろはにほへど ちりぬるを\n" "わがよたれぞ つねならむ\n" "うゐのおくやま けふこえて\n" "あさきゆめみじ ゑひもせず", + "hiragana": "いろはにほへど ちりぬるを\nわがよたれぞ つねならむ\nうゐのおくやま けふこえて\nあさきゆめみじ ゑひもせず", "poo": "Hello, \U0001f4a9!", } diff --git a/tox.ini b/tox.ini index 0fe6f230..18c5ff84 100644 --- a/tox.ini +++ b/tox.ini @@ -54,26 +54,16 @@ commands= [testenv:check] skip_install = true +deps = ruff commands = - black --check {toxinidir} - isort --check --df {toxinidir} - flake8 {toxinidir} + ruff check {toxinidir} + ruff format --check {toxinidir} [testenv:mypy] commands = mypy --config {toxinidir}/mypi.ini {toxinidir}/src -[flake8] -max-line-length = 160 -extend-ignore = E203 -# test_github.py ignored because of https://gitlab.com/pycqa/flake8/issues/375 -exclude = .ropeproject,.tox,sandbox,docs,.eggs,*.egg,*.egg-info,setup.py,build/,tests/test_github.py,.venv -per-file-ignores = - # Ignore documentation issues in tests - tests:*:D -show-source = True - [pytest] norecursedirs = .tox .git .hg sandbox .eggs build python_files = test_*.py