From 1e353adff719bcc1e868f84899dcdef6b020f5e7 Mon Sep 17 00:00:00 2001 From: Tim Pillinger <26465611+wxtim@users.noreply.github.com> Date: Thu, 14 Dec 2023 11:11:05 +0000 Subject: [PATCH 1/5] lint.warn about deprecated templating --- changes.d/5879.feat.md | 1 + cylc/flow/scripts/lint.py | 72 ++++++++++++++++++++++++++++++++- tests/unit/scripts/test_lint.py | 2 +- 3 files changed, 73 insertions(+), 2 deletions(-) create mode 100644 changes.d/5879.feat.md diff --git a/changes.d/5879.feat.md b/changes.d/5879.feat.md new file mode 100644 index 00000000000..ba50091b19a --- /dev/null +++ b/changes.d/5879.feat.md @@ -0,0 +1 @@ +Warn of use of old templated items such as %(suite)s diff --git a/cylc/flow/scripts/lint.py b/cylc/flow/scripts/lint.py index cc06512988c..934c9124793 100755 --- a/cylc/flow/scripts/lint.py +++ b/cylc/flow/scripts/lint.py @@ -67,7 +67,7 @@ TOMLDecodeError, ) from typing import ( - TYPE_CHECKING, Any, Callable, Dict, Iterator, List, Union) + TYPE_CHECKING, Any, Callable, Dict, Iterator, List, Optional, Union) from cylc.flow import LOG from cylc.flow.exceptions import CylcError @@ -106,6 +106,29 @@ } +DEPRECATED_STRING_TEMPLATES = { + 'suite': 'workflow', + 'suite_uuid': 'uuid', + 'batch_sys_name': 'job_runner_name', + 'batch_sys_job_id': 'job_id', + 'user@host': 'platform_name', + 'task_url': 'URL from task metadata', + 'workflow_url': 'workflow_URL from workflow metadata', +} + + +LIST_ITEM = ' * ' + + +deprecated_string_templates = { + key: ( + re.compile(r'%\(' + key + r'\)s'), + value + ) + for key, value in DEPRECATED_STRING_TEMPLATES.items() +} + + def check_jinja2_no_shebang( line: str, file: Path, @@ -233,6 +256,36 @@ def check_for_obsolete_environment_variables(line: str) -> List[str]: return [i for i in OBSOLETE_ENV_VARS if i in line] +def check_for_deprecated_task_event_template_vars( + line: str +) -> Optional[Dict[str, str]]: + """Look for string variables which are no longer supported + + Examples: + >>> this = check_for_deprecated_task_event_template_vars + + >>> this('hello = "My name is %(suite)s"') + {'list': ' * %(suite)s ⇒ %(workflow)s'} + + >>> expect = {'list': ( + ... ' * %(suite)s ⇒ %(workflow)s' + ... ' * %(task_url)s - get URL from task metadata')} + >>> this('hello = "My name is %(suite)s, %(task_url)s"') == expect + True + """ + result = [] + for key, (regex, replacement) in deprecated_string_templates.items(): + search_outcome = regex.findall(line) + if search_outcome and ' ' in replacement: + result.append(f'%({key})s - get {replacement}') + elif search_outcome: + result.append(f'%({key})s ⇒ %({replacement})s') + + if result: + return {'list': LIST_ITEM + LIST_ITEM.join(result)} + return None + + INDENTATION = re.compile(r'^(\s*)(.*)') @@ -584,6 +637,23 @@ def check_indentation(line: str) -> bool: ), FUNCTION: re.compile(r'rose +date').findall, }, + 'U015': { + 'short': ( + 'Deprecated template variables.\n{list}'), + 'rst': ( + 'The following template variables, mostly used in event handlers,' + 'are deprecated, and should be replaced:' + + ''.join([ + f'\n * ``{old} ⇒ {new}' + for old, new in DEPRECATED_STRING_TEMPLATES.items() + ]) + ), + 'url': ( + 'https://cylc.github.io/cylc-doc/stable/html/user-guide/' + 'writing-workflows/runtime.html#task-event-template-variables' + ), + FUNCTION: check_for_deprecated_task_event_template_vars, + } } RULESETS = ['728', 'style', 'all'] EXTRA_TOML_VALIDATION = { diff --git a/tests/unit/scripts/test_lint.py b/tests/unit/scripts/test_lint.py index 6ee1018cf96..349854be312 100644 --- a/tests/unit/scripts/test_lint.py +++ b/tests/unit/scripts/test_lint.py @@ -119,7 +119,7 @@ submission failed handler = giaSEHFUIHJ failed handler = woo execution timeout handler = sdfghjkl - expired handler = dafuhj + expired handler = %(suite_uuid)s %(user@host)s late handler = dafuhj submitted handler = dafuhj started handler = dafuhj From c113906569c2845c842857e03c4d30b096479109 Mon Sep 17 00:00:00 2001 From: Tim Pillinger <26465611+wxtim@users.noreply.github.com> Date: Thu, 14 Dec 2023 15:16:48 +0000 Subject: [PATCH 2/5] Update changes.d/5879.feat.md Co-authored-by: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com> --- changes.d/5879.feat.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changes.d/5879.feat.md b/changes.d/5879.feat.md index ba50091b19a..be4c7e14e94 100644 --- a/changes.d/5879.feat.md +++ b/changes.d/5879.feat.md @@ -1 +1 @@ -Warn of use of old templated items such as %(suite)s +`cylc lint` now warns of use of old templated items such as `%(suite)s` From 19a9e9e7e37d23a18573f0e628ac97f5e0f1bc6d Mon Sep 17 00:00:00 2001 From: Tim Pillinger <26465611+wxtim@users.noreply.github.com> Date: Thu, 14 Dec 2023 15:19:40 +0000 Subject: [PATCH 3/5] Update cylc/flow/scripts/lint.py Co-authored-by: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com> --- cylc/flow/scripts/lint.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cylc/flow/scripts/lint.py b/cylc/flow/scripts/lint.py index 934c9124793..82e9643d262 100755 --- a/cylc/flow/scripts/lint.py +++ b/cylc/flow/scripts/lint.py @@ -644,7 +644,7 @@ def check_indentation(line: str) -> bool: 'The following template variables, mostly used in event handlers,' 'are deprecated, and should be replaced:' + ''.join([ - f'\n * ``{old} ⇒ {new}' + f'\n * ``{old}`` ⇒ ``{new}``' for old, new in DEPRECATED_STRING_TEMPLATES.items() ]) ), From b7c0b939e0d98c71262507dba8dcbf9e8531e32a Mon Sep 17 00:00:00 2001 From: Tim Pillinger <26465611+wxtim@users.noreply.github.com> Date: Thu, 14 Dec 2023 17:19:01 +0000 Subject: [PATCH 4/5] Clarify templated vars which have to be set in metadata sections. Fix an rst bug. --- cylc/flow/scripts/lint.py | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/cylc/flow/scripts/lint.py b/cylc/flow/scripts/lint.py index 82e9643d262..240f3decf02 100755 --- a/cylc/flow/scripts/lint.py +++ b/cylc/flow/scripts/lint.py @@ -112,8 +112,10 @@ 'batch_sys_name': 'job_runner_name', 'batch_sys_job_id': 'job_id', 'user@host': 'platform_name', - 'task_url': 'URL from task metadata', - 'workflow_url': 'workflow_URL from workflow metadata', + 'task_url': '``URL`` (if set in :cylc:conf:`[meta]URL`)', + 'workflow_url': ( + '``workflow_URL`` (if set in ' + ':cylc:conf:`[runtime][][meta]URL`)'), } @@ -268,8 +270,8 @@ def check_for_deprecated_task_event_template_vars( {'list': ' * %(suite)s ⇒ %(workflow)s'} >>> expect = {'list': ( - ... ' * %(suite)s ⇒ %(workflow)s' - ... ' * %(task_url)s - get URL from task metadata')} + ... ' * %(suite)s ⇒ %(workflow)s * %(task_url)s' + ... ' - get ``URL`` (if set in :cylc:conf:`[meta]URL`)')} >>> this('hello = "My name is %(suite)s, %(task_url)s"') == expect True """ @@ -639,12 +641,12 @@ def check_indentation(line: str) -> bool: }, 'U015': { 'short': ( - 'Deprecated template variables.\n{list}'), + 'Deprecated template variables.'), 'rst': ( 'The following template variables, mostly used in event handlers,' 'are deprecated, and should be replaced:' + ''.join([ - f'\n * ``{old}`` ⇒ ``{new}``' + f'\n * ``{old}`` ⇒ {new}' for old, new in DEPRECATED_STRING_TEMPLATES.items() ]) ), @@ -1147,7 +1149,7 @@ def get_cylc_files( 'section heading': '\n{title}\n{underline}\n', 'issue heading': { 'text': '\n{check}:\n {summary}\n {url}\n\n', - 'rst': '\n`{check} <{url}>`_\n{underline}\n{summary}\n\n', + 'rst': '\n{url_block}_\n{underline}\n{summary}\n\n', }, 'auto gen message': ( 'U998 and U999 represent automatically generated' @@ -1194,11 +1196,12 @@ def get_reference(linter, output_type): else: template = issue_heading_template url = get_url(meta) + check = get_index_str(meta, index) msg = template.format( title=index, - check=get_index_str(meta, index), + check=check, summary=summary, - url=url, + url_block=f'`{check} <{url}>`' if url else f'{check}', underline=( len(get_index_str(meta, index)) + len(url) + 6 ) * '^' From 248cb7ed162499b7d1dfa8b079eb7ddbbc12709e Mon Sep 17 00:00:00 2001 From: Tim Pillinger <26465611+wxtim@users.noreply.github.com> Date: Fri, 15 Dec 2023 16:28:36 +0000 Subject: [PATCH 5/5] fix broken code --- cylc/flow/scripts/lint.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/cylc/flow/scripts/lint.py b/cylc/flow/scripts/lint.py index 240f3decf02..ef7635f1f0d 100755 --- a/cylc/flow/scripts/lint.py +++ b/cylc/flow/scripts/lint.py @@ -1149,7 +1149,7 @@ def get_cylc_files( 'section heading': '\n{title}\n{underline}\n', 'issue heading': { 'text': '\n{check}:\n {summary}\n {url}\n\n', - 'rst': '\n{url_block}_\n{underline}\n{summary}\n\n', + 'rst': '\n{url}_\n{underline}\n{summary}\n\n', }, 'auto gen message': ( 'U998 and U999 represent automatically generated' @@ -1194,17 +1194,17 @@ def get_reference(linter, output_type): output += '\n' output += '\n* ' + summary else: + check = get_index_str(meta, index) template = issue_heading_template url = get_url(meta) - check = get_index_str(meta, index) + if output_type == 'rst': + url = f'`{check} <{url}>`' if url else f'{check}' msg = template.format( title=index, check=check, summary=summary, - url_block=f'`{check} <{url}>`' if url else f'{check}', - underline=( - len(get_index_str(meta, index)) + len(url) + 6 - ) * '^' + url=url, + underline=(len(url) + 1) * '^' ) output += msg output += '\n'