Skip to content

Commit

Permalink
increase worflow linting
Browse files Browse the repository at this point in the history
  • Loading branch information
lldelisle committed Jun 29, 2024
1 parent 9ab4982 commit 64c684c
Show file tree
Hide file tree
Showing 3 changed files with 96 additions and 4 deletions.
6 changes: 6 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 Down
2 changes: 2 additions & 0 deletions planemo/lint.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ def build_lint_args(ctx, **kwds):
"""Handle common report, error, and skip linting arguments."""
report_level = kwds.get("report_level", "all")
fail_level = kwds.get("fail_level", "warn")
iwc_grade = kwds.get("iwc", False)
skip = kwds.get("skip", None)
if skip is None:
skip = ctx.global_config.get("lint_skip", "")
Expand All @@ -42,6 +43,7 @@ def build_lint_args(ctx, **kwds):
level=report_level,
fail_level=fail_level,
skip_types=skip_types,
iwc_grade=iwc_grade,
)
return lint_args

Expand Down
92 changes: 88 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
iwc_grade = False


def generate_dockstore_yaml(directory: str, publish: bool = True) -> str:
Expand Down Expand Up @@ -138,6 +140,13 @@ 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_args['iwc_grade']:
if not os.path.isdir(path):
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)
lint_context.iwc_grade = True

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 @@ -150,8 +159,10 @@ def structure(path, lint_context):
workflow_class = workflow_dict.get("class")
lint_func = lint_format2 if workflow_class == "GalaxyWorkflow" else lint_ga
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 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 @@ -415,20 +439,26 @@ def _lint_dockstore_workflow_entry(
return

found_errors = False
for required_key in ["primaryDescriptorPath", "subclass"]:
for required_key in ["primaryDescriptorPath", "subclass", "name"]:
if required_key not in workflow_entry:
lint_context.error(f"{DOCKSTORE_REGISTRY_CONF} workflow entry missing required key {required_key}")
found_errors = True

if lint_context.iwc_grade:
lint_fun = lint_context.error
else:
lint_fun = lint_context.warn
for recommended_key in ["testParameterFiles"]:
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,24 @@ 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} should have no " \
"'authors' in the .dockstore.yml.")
# Check there is not mailto
for author in workflow_entry.get('authors'):
if 'email' in author:
if author['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 +585,39 @@ 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 {file} does not match " \
"the version in the CHANGELOG.")

0 comments on commit 64c684c

Please sign in to comment.