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

Increase worflow linting #1463

Merged
merged 29 commits into from
Jul 4, 2024
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
64c684c
increase worflow linting
lldelisle Jun 29, 2024
af6758e
linting
lldelisle Jun 29, 2024
1d61ca1
restore build_lint_args
lldelisle Jun 29, 2024
dd9b1bb
fix cases with no authors in dockstore.yml
lldelisle Jun 29, 2024
f7401d8
fix DOCKSTORE_REGISTRY_CONF_VERSION
lldelisle Jun 29, 2024
d4ae50f
change name from required to recommanded
lldelisle Jun 29, 2024
619cae9
linting
lldelisle Jun 29, 2024
eb86e3b
fix forgotten consequence of name from required to recommanded
lldelisle Jun 29, 2024
5516443
put iwc_grade in lint_args as suggested by @bernt-matthias
lldelisle Jul 1, 2024
8639d1d
remove release/version from WorkflowLintContext as suggested by @matt…
lldelisle Jul 1, 2024
5c19e56
fix type of lint_args
lldelisle Jul 1, 2024
302c504
remove iwc_grade from WorkflowLintContext and add lint_dockstore_best…
lldelisle Jul 1, 2024
1310eb7
new build_wf_lint_args to match what is happening with tools
lldelisle Jul 1, 2024
906c5ca
fix error on CHANGELOG get version
lldelisle Jul 1, 2024
39480a8
add tests
lldelisle Jul 1, 2024
7bb3a50
restore comma
lldelisle Jul 1, 2024
db8f24c
test --iwc on file
lldelisle Jul 1, 2024
22112e8
remove path from asserts output
lldelisle Jul 1, 2024
147296a
run black
lldelisle Jul 1, 2024
f5af20f
run isort
lldelisle Jul 1, 2024
6e8f0ce
fix strange output when no --skip
lldelisle Jul 1, 2024
f748c11
fix final test
lldelisle Jul 1, 2024
0d8dbd6
change strategy for iwc on files
lldelisle Jul 1, 2024
f88024c
Merge remote-tracking branch 'upstream/master' into lint_workflow
lldelisle Jul 1, 2024
64f9076
add comma
lldelisle Jul 1, 2024
bafecff
Add type annotation
nsoranzo Jul 2, 2024
b1eedf4
Apply suggestions from @bernt-matthias
lldelisle Jul 2, 2024
ecdcf0f
fix lint_args typing to different places
lldelisle Jul 2, 2024
2f7c139
remove unused Union
lldelisle Jul 2, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions planemo/commands/cmd_workflow_lint.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,12 @@
@options.report_xunit()
@options.fail_level_option()
@options.skip_option()
@click.option(
"--iwc",
is_flag=True,
default=False,
help="Check workflows directory with the standards of iwc",
)
@command_function
def cli(ctx, paths, **kwds):
"""Check workflows for syntax errors and best practices."""
Expand All @@ -23,5 +29,6 @@ def cli(ctx, paths, **kwds):
ctx,
paths,
lint_args,
iwc_grade=kwds.get("iwc", False)
lldelisle marked this conversation as resolved.
Show resolved Hide resolved
)
ctx.exit(exit_code)
93 changes: 89 additions & 4 deletions planemo/workflow_lint.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ class WorkflowLintContext(LintContext):
# Setup training topic for linting - probably should pass this through
# from click arguments.
training_topic = None
release = None
lldelisle marked this conversation as resolved.
Show resolved Hide resolved
iwc_grade = False


def generate_dockstore_yaml(directory: str, publish: bool = True) -> str:
Expand Down Expand Up @@ -122,10 +124,11 @@ def generate_dockstore_yaml(directory: str, publish: bool = True) -> str:


def lint_workflow_artifacts_on_paths(
ctx: "PlanemoCliContext", paths: Iterable[str], lint_args: Dict[str, Union[str, List[str]]]
ctx: "PlanemoCliContext", paths: Iterable[str], lint_args: Dict[str, Union[str, List[str]]], iwc_grade: bool
) -> int:
report_level = lint_args["level"]
lint_context = WorkflowLintContext(report_level, skip_types=lint_args["skip_types"])
lint_context.iwc_grade = iwc_grade
for path in paths:
_lint_workflow_artifacts_on_path(lint_context, path, lint_args)

Expand All @@ -138,6 +141,12 @@ def lint_workflow_artifacts_on_paths(
def _lint_workflow_artifacts_on_path(
lint_context: WorkflowLintContext, path: str, lint_args: Dict[str, Union[str, List[str]]]
) -> None:
if lint_context.iwc_grade:
if not os.path.isdir(path):
lldelisle marked this conversation as resolved.
Show resolved Hide resolved
raise ValueError("iwc standards can only be checked on directories.")
lint_context.lint("lint_required_files", _lint_required_files_workflow_dir, path)
lint_context.lint("lint_changelog", _lint_changelog_version, path)

for potential_workflow_artifact_path in find_potential_workflow_files(path):
if os.path.basename(potential_workflow_artifact_path) == DOCKSTORE_REGISTRY_CONF:
lint_context.lint("lint_dockstore", _lint_dockstore_config, potential_workflow_artifact_path)
Expand All @@ -152,6 +161,8 @@ def structure(path, lint_context):
lint_func(lint_context, workflow_dict, path=path)

lint_context.lint("lint_structure", structure, potential_workflow_artifact_path)
if lint_context.iwc_grade and lint_context.version is not None:
lint_context.lint("lint_version", check_version, potential_workflow_artifact_path)
lint_context.lint("lint_best_practices", _lint_best_practices, potential_workflow_artifact_path)
lint_context.lint("lint_tests", _lint_tsts, potential_workflow_artifact_path)
lint_context.lint("lint_tool_ids", _lint_tool_ids, potential_workflow_artifact_path)
Expand Down Expand Up @@ -394,6 +405,11 @@ def _lint_dockstore_config(path: str, lint_context: WorkflowLintContext) -> None
lint_context.error("Invalid YAML contents found in %s" % DOCKSTORE_REGISTRY_CONF)
return

if "version" not in dockstore_yaml:
lint_context.error("Invalid YAML contents found in %s, no version defined" % DOCKSTORE_REGISTRY_CONF)
if str(dockstore_yaml.get("version")) != DOCKSTORE_REGISTRY_CONF_VERSION:
lint_context.error("Invalid YAML version found in %s." % DOCKSTORE_REGISTRY_CONF)

if "workflows" not in dockstore_yaml:
lint_context.error("Invalid YAML contents found in %s, no workflows defined" % DOCKSTORE_REGISTRY_CONF)
return
Expand All @@ -403,8 +419,16 @@ def _lint_dockstore_config(path: str, lint_context: WorkflowLintContext) -> None
lint_context.error("Invalid YAML contents found in %s, workflows not a list" % DOCKSTORE_REGISTRY_CONF)
return

if len(workflow_entries) == 0:
lint_context.error("No workflow specified in the .dockstore.yml.")

workflow_names_in_dockstore = []
for workflow_entry in workflow_entries:
_lint_dockstore_workflow_entry(lint_context, os.path.dirname(path), workflow_entry)
workflow_name = workflow_entry.get("name", "")
if workflow_name in workflow_names_in_dockstore:
lint_context.error(f"{DOCKSTORE_REGISTRY_CONF} has multiple workflow entries with the same name")
workflow_names_in_dockstore.append(workflow_name)


def _lint_dockstore_workflow_entry(
Expand All @@ -420,15 +444,21 @@ def _lint_dockstore_workflow_entry(
lint_context.error(f"{DOCKSTORE_REGISTRY_CONF} workflow entry missing required key {required_key}")
found_errors = True

for recommended_key in ["testParameterFiles"]:
if lint_context.iwc_grade:
lldelisle marked this conversation as resolved.
Show resolved Hide resolved
lint_fun = lint_context.error
else:
lint_fun = lint_context.warn
for recommended_key in ["testParameterFiles", "name"]:
if recommended_key not in workflow_entry:
lint_context.warn(f"{DOCKSTORE_REGISTRY_CONF} workflow entry missing recommended key {recommended_key}")
lint_fun(f"{DOCKSTORE_REGISTRY_CONF} workflow entry missing recommended key {recommended_key}")

if found_errors:
# Don't do the rest of the validation for a broken file.
return

# TODO: validate subclass
if workflow_entry.get("subclass") != "Galaxy":
lint_context.error(f"{DOCKSTORE_REGISTRY_CONF} workflow entry subclass must be 'Galaxy'.")

descriptor_path = workflow_entry["primaryDescriptorPath"]
test_files = workflow_entry.get("testParameterFiles", [])

Expand All @@ -437,6 +467,23 @@ def _lint_dockstore_workflow_entry(
if not os.path.exists(referenced_path):
lint_context.error(f"{DOCKSTORE_REGISTRY_CONF} workflow entry references absent file {referenced_file}")

# Check there is no space in name:
workflow_name = workflow_entry.get("name", "")
# Check the name has no space
if ' ' in workflow_name:
lint_context.error("Dockstore does not accept workflow names with space.",
f"Change '{workflow_name}' in {DOCKSTORE_REGISTRY_CONF}.")

# Check there is at least one author
if len(workflow_entry.get('authors', [])) == 0:
lint_fun(f"Workflow {workflow_name} have no "
"'authors' in the .dockstore.yml.")
# Check there is not mailto
for author in workflow_entry.get('authors', []):
if author.get('email', '').startswith('mailto:'):
lint_context.error("email field of the .dockstore.yml must not "
"contain 'mailto:'")


def looks_like_a_workflow(path: str) -> bool:
"""Return boolean indicating if this path looks like a workflow."""
Expand Down Expand Up @@ -537,3 +584,41 @@ def _lint_tool_ids_steps(lint_context: WorkflowLintContext, wf_dict: Dict, ts: T
if not failed:
lint_context.valid("All tool ids appear to be valid.")
return None


def _lint_required_files_workflow_dir(path: str, lint_context: WorkflowLintContext) -> None:
# Check all required files are present
required_files = \
['README.md', 'CHANGELOG.md', '.dockstore.yml']
for required_file in required_files:
if not os.path.exists(os.path.join(path, required_file)):
lint_context.error(f"The file {required_file} is"
" missing but required.")


def _lint_changelog_version(path: str, lint_context: WorkflowLintContext) -> None:
lint_context.version = None
# Check the version can be get from the CHANGELOG.md
if not os.path.exists(os.path.join(path, 'CHANGELOG.md')):
return
with open(os.path.join(path, 'CHANGELOG.md'), 'r') as f:
for line in f:
if line.startswith('## ['):
lint_context.version = line.split(']')[0].replace('## [', '')
break

if lint_context.version is None:
lint_context.error("No version found in CHANGELOG. "
"The version should be in a line that starts like "
"'## [version number]'")


def check_version(path, lint_context):
with open(path) as f:
workflow_dict = ordered_load(f)
if 'release' not in workflow_dict:
lint_context.error(f"The workflow {path} has no release")
else:
if lint_context.version is not None and workflow_dict.get("release") != lint_context.version:
lint_context.error(f"The release of workflow {path} does not match "
"the version in the CHANGELOG.")
Loading