Skip to content

Commit

Permalink
Merge branch 'dev' into singularity_registry
Browse files Browse the repository at this point in the history
drpatelh authored Jun 15, 2023
2 parents 49c793c + 3de58f2 commit 8c96eb4
Showing 7 changed files with 83 additions and 33 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -12,7 +12,10 @@
- Remove `aws_tower` profile ([#2287])(https://github.com/nf-core/tools/pull/2287)
- Fixed the Slack report to include the pipeline name ([#2291](https://github.com/nf-core/tools/pull/2291))
- Fix link in the MultiQC report to point to exact version of output docs ([#2298](https://github.com/nf-core/tools/pull/2298))
- Updates seqeralabs/action-tower-launch to v2.0.0 ([#2301](https://github.com/nf-core/tools/pull/2301))
- Remove schema validation from `lib` folder and use Nextflow [nf-validation plugin](https://nextflow-io.github.io/nf-validation/) instead ([#1771](https://github.com/nf-core/tools/pull/1771/))
- Fix parsing of container directive when it is not typical nf-core format ([#2306](https://github.com/nf-core/tools/pull/2306))
- Add ability to specify custom registry for linting modules, defaults to quay.io ([#2313](https://github.com/nf-core/tools/pull/2313))
- Add `singularity.registry = 'quay.io'` in pipeline template
- Bump minimum required NF version in pipeline template from `22.10.1` -> `23.04.0`

6 changes: 5 additions & 1 deletion nf_core/__main__.py
Original file line number Diff line number Diff line change
@@ -807,6 +807,9 @@ def create_test_yml(ctx, tool, run_tests, output, force, no_prompts):
@click.pass_context
@click.argument("tool", type=str, required=False, metavar="<tool> or <tool/subtool>")
@click.option("-d", "--dir", type=click.Path(exists=True), default=".", metavar="<pipeline/modules directory>")
@click.option(
"-r", "--registry", type=str, metavar="<registry>", default="quay.io", help="Registry to use for containers"
)
@click.option("-k", "--key", type=str, metavar="<test>", multiple=True, help="Run only these lint tests")
@click.option("-a", "--all", is_flag=True, help="Run on all modules")
@click.option("-w", "--fail-warned", is_flag=True, help="Convert warn tests to failures")
@@ -821,7 +824,7 @@ def create_test_yml(ctx, tool, run_tests, output, force, no_prompts):
)
@click.option("--fix-version", is_flag=True, help="Fix the module version if a newer version is available")
def lint(
ctx, tool, dir, key, all, fail_warned, local, passed, sort_by, fix_version
ctx, tool, dir, registry, key, all, fail_warned, local, passed, sort_by, fix_version
): # pylint: disable=redefined-outer-name
"""
Lint one or more modules in a directory.
@@ -846,6 +849,7 @@ def lint(
)
module_lint.lint(
module=tool,
registry=registry,
key=key,
all_modules=all,
print_results=True,
16 changes: 9 additions & 7 deletions nf_core/modules/lint/__init__.py
Original file line number Diff line number Diff line change
@@ -145,6 +145,7 @@ def get_all_lint_tests(is_pipeline):
def lint(
self,
module=None,
registry="quay.io",
key=(),
all_modules=False,
print_results=True,
@@ -227,11 +228,11 @@ def lint(

# Lint local modules
if local and len(local_modules) > 0:
self.lint_modules(local_modules, local=True, fix_version=fix_version)
self.lint_modules(local_modules, registry=registry, local=True, fix_version=fix_version)

# Lint nf-core modules
if len(remote_modules) > 0:
self.lint_modules(remote_modules, local=False, fix_version=fix_version)
self.lint_modules(remote_modules, registry=registry, local=False, fix_version=fix_version)

if print_results:
self._print_results(show_passed=show_passed, sort_by=sort_by)
@@ -264,12 +265,13 @@ def filter_tests_by_key(self, key):
# If -k supplied, only run these tests
self.lint_tests = [k for k in self.lint_tests if k in key]

def lint_modules(self, modules, local=False, fix_version=False):
def lint_modules(self, modules, registry="quay.io", local=False, fix_version=False):
"""
Lint a list of modules
Args:
modules ([NFCoreModule]): A list of module objects
registry (str): The container registry to use. Should be quay.io in most situations.
local (boolean): Whether the list consist of local or nf-core modules
fix_version (boolean): Fix the module version if a newer version is available
"""
@@ -290,9 +292,9 @@ def lint_modules(self, modules, local=False, fix_version=False):

for mod in modules:
progress_bar.update(lint_progress, advance=1, test_name=mod.module_name)
self.lint_module(mod, progress_bar, local=local, fix_version=fix_version)
self.lint_module(mod, progress_bar, registry=registry, local=local, fix_version=fix_version)

def lint_module(self, mod, progress_bar, local=False, fix_version=False):
def lint_module(self, mod, progress_bar, registry, local=False, fix_version=False):
"""
Perform linting on one module
@@ -311,7 +313,7 @@ def lint_module(self, mod, progress_bar, local=False, fix_version=False):

# Only check the main script in case of a local module
if local:
self.main_nf(mod, fix_version, progress_bar)
self.main_nf(mod, fix_version, registry, progress_bar)
self.passed += [LintResult(mod, *m) for m in mod.passed]
warned = [LintResult(mod, *m) for m in (mod.warned + mod.failed)]
if not self.fail_warned:
@@ -323,7 +325,7 @@ def lint_module(self, mod, progress_bar, local=False, fix_version=False):
else:
for test_name in self.lint_tests:
if test_name == "main_nf":
getattr(self, test_name)(mod, fix_version, progress_bar)
getattr(self, test_name)(mod, fix_version, registry, progress_bar)
else:
getattr(self, test_name)(mod)

60 changes: 39 additions & 21 deletions nf_core/modules/lint/main_nf.py
Original file line number Diff line number Diff line change
@@ -17,7 +17,7 @@
log = logging.getLogger(__name__)


def main_nf(module_lint_object, module, fix_version, progress_bar):
def main_nf(module_lint_object, module, fix_version, registry, progress_bar):
"""
Lint a ``main.nf`` module file
@@ -121,7 +121,7 @@ def main_nf(module_lint_object, module, fix_version, progress_bar):
module.passed.append(("main_nf_script_outputs", "Process 'output' block found", module.main_nf))

# Check the process definitions
if check_process_section(module, process_lines, fix_version, progress_bar):
if check_process_section(module, process_lines, registry, fix_version, progress_bar):
module.passed.append(("main_nf_container", "Container versions match", module.main_nf))
else:
module.warned.append(("main_nf_container", "Container versions do not match", module.main_nf))
@@ -209,12 +209,20 @@ def check_when_section(self, lines):
self.passed.append(("when_condition", "when: condition is unchanged", self.main_nf))


def check_process_section(self, lines, fix_version, progress_bar):
"""
Lint the section of a module between the process definition
def check_process_section(self, lines, registry, fix_version, progress_bar):
"""Lint the section of a module between the process definition
and the 'input:' definition
Specifically checks for correct software versions
and containers
Args:
lines (List[str]): Content of process.
registry (str): Base Docker registry for containers. Typically quay.io.
fix_version (bool): Fix software version
progress_bar (ProgressBar): Progress bar to update.
Returns:
Optional[bool]: True if singularity and docker containers match, False otherwise. If process definition does not exist, None.
"""
# Check that we have a process section
if len(lines) == 0:
@@ -223,8 +231,8 @@ def check_process_section(self, lines, fix_version, progress_bar):
self.passed.append(("process_exist", "Process definition exists", self.main_nf))

# Checks that build numbers of bioconda, singularity and docker container are matching
singularity_tag = "singularity"
docker_tag = "docker"
singularity_tag = None
docker_tag = None
bioconda_packages = []

# Process name should be all capital letters
@@ -240,7 +248,12 @@ def check_process_section(self, lines, fix_version, progress_bar):
# Deprecated enable_conda
for i, l in enumerate(lines):
url = None
l = l.strip(" '\"")
l = l.strip(" \n'\"}:")

# Catch preceeding "container "
if l.startswith("container"):
l = l.replace("container", "").strip(" \n'\"}:")

if _container_type(l) == "conda":
bioconda_packages = [b for b in l.split() if "bioconda::" in b]
match = re.search(r"params\.enable_conda", l)
@@ -261,9 +274,10 @@ def check_process_section(self, lines, fix_version, progress_bar):
)
)
if _container_type(l) == "singularity":
# e.g. "https://containers.biocontainers.pro/s3/SingImgsRepo/biocontainers/v1.2.0_cv1/biocontainers_v1.2.0_cv1.img' :" -> v1.2.0_cv1
# e.g. "https://depot.galaxyproject.org/singularity/fastqc:0.11.9--0' :" -> 0.11.9--0
match = re.search(r"(?:/)?(?:biocontainers_)?(?::)?([A-Za-z\d\-_.]+?)(?:\.img)?'", l)
# e.g. "https://containers.biocontainers.pro/s3/SingImgsRepo/biocontainers/v1.2.0_cv1/biocontainers_v1.2.0_cv1.img -> v1.2.0_cv1
# e.g. "https://depot.galaxyproject.org/singularity/fastqc:0.11.9--0 -> 0.11.9--0
# Please god let's find a better way to do this than regex
match = re.search(r"(?:[:.])?([A-Za-z\d\-_.]+?)(?:\.img)?(?:\.sif)?$", l)
if match is not None:
singularity_tag = match.group(1)
self.passed.append(("singularity_tag", f"Found singularity tag: {singularity_tag}", self.main_nf))
@@ -273,16 +287,16 @@ def check_process_section(self, lines, fix_version, progress_bar):
url = urlparse(l.split("'")[0])

if _container_type(l) == "docker":
# e.g. "quay.io/biocontainers/krona:2.7.1--pl526_5' }" -> 2.7.1--pl526_5
# e.g. "biocontainers/biocontainers:v1.2.0_cv1' }" -> v1.2.0_cv1
match = re.search(r"(?:[/])?(?::)?([A-Za-z\d\-_.]+)'", l)
# e.g. "quay.io/biocontainers/krona:2.7.1--pl526_5 -> 2.7.1--pl526_5
# e.g. "biocontainers/biocontainers:v1.2.0_cv1 -> v1.2.0_cv1
match = re.search(r":([A-Za-z\d\-_.]+)$", l)
if match is not None:
docker_tag = match.group(1)
self.passed.append(("docker_tag", f"Found docker tag: {docker_tag}", self.main_nf))
else:
self.failed.append(("docker_tag", "Unable to parse docker tag", self.main_nf))
docker_tag = NoneD
if l.startswith("quay.io/"):
docker_tag = None
if l.startswith(registry):
l_stripped = re.sub(r"\W+$", "", l)
self.failed.append(
(
@@ -297,7 +311,7 @@ def check_process_section(self, lines, fix_version, progress_bar):
# Guess if container name is simple one (e.g. nfcore/ubuntu:20.04)
# If so, add quay.io as default container prefix
if l.count("/") == 1 and l.count(":") == 1:
l = "quay.io/" + l
l = "/".join([registry, l]).replace("//", "/")
url = urlparse(l.split("'")[0])

# lint double quotes
@@ -320,7 +334,7 @@ def check_process_section(self, lines, fix_version, progress_bar):
)

# lint more than one container in the same line
if ("https://containers" in l or "https://depot" in l) and ("biocontainers/" in l or "quay.io/" in l):
if ("https://containers" in l or "https://depot" in l) and ("biocontainers/" in l or l.startswith(registry)):
self.warned.append(
(
"container_links",
@@ -412,7 +426,11 @@ def check_process_section(self, lines, fix_version, progress_bar):
else:
self.passed.append(("bioconda_latest", f"Conda package is the latest available: `{bp}`", self.main_nf))

return docker_tag == singularity_tag
# Check if a tag exists at all. If not, return None.
if singularity_tag is None or docker_tag is None:
return None
else:
return docker_tag == singularity_tag


def check_process_labels(self, lines):
@@ -591,7 +609,7 @@ def _container_type(line):
"""Returns the container type of a build."""
if line.startswith("conda"):
return "conda"
if line.startswith("https://containers") or line.startswith("https://depot"):
if line.startswith("https://") or line.startswith("https://depot"):
# Look for a http download URL.
# Thanks Stack Overflow for the regex: https://stackoverflow.com/a/3809435/713980
url_regex = (
@@ -601,5 +619,5 @@ def _container_type(line):
if url_match:
return "singularity"
return None
if line.count("/") >= 1 and line.count(":") == 1:
if line.count("/") >= 1 and line.count(":") == 1 and line.count(" ") == 0 and "https://" not in line:
return "docker"
9 changes: 7 additions & 2 deletions nf_core/pipeline-template/.github/workflows/awsfulltest.yml
Original file line number Diff line number Diff line change
@@ -14,21 +14,26 @@ jobs:
runs-on: ubuntu-latest
steps:
- name: Launch workflow via tower
uses: seqeralabs/action-tower-launch@v1
uses: seqeralabs/action-tower-launch@v2
# TODO nf-core: You can customise AWS full pipeline tests as required
# Add full size test data (but still relatively small datasets for few samples)
# on the `test_full.config` test runs with only one set of parameters {%- raw %}
with:
workspace_id: ${{ secrets.TOWER_WORKSPACE_ID }}
access_token: ${{ secrets.TOWER_ACCESS_TOKEN }}
compute_env: ${{ secrets.TOWER_COMPUTE_ENV }}
revision: ${{ github.sha }}
workdir: s3://${{ secrets.AWS_S3_BUCKET }}{% endraw %}/work/{{ short_name }}/{% raw %}work-${{ github.sha }}{% endraw %}
parameters: |
{
"hook_url": "{% raw %}${{ secrets.MEGATESTS_ALERTS_SLACK_HOOK_URL }}{% endraw %}",
"outdir": "s3://{% raw %}${{ secrets.AWS_S3_BUCKET }}{% endraw %}/{{ short_name }}/{% raw %}results-${{ github.sha }}{% endraw %}"
}
profiles: test_full

- uses: actions/upload-artifact@v3
with:
name: Tower debug log file
path: tower_action_*.log
path: |
tower_action_*.log
tower_action_*.json
8 changes: 6 additions & 2 deletions nf_core/pipeline-template/.github/workflows/awstest.yml
Original file line number Diff line number Diff line change
@@ -12,18 +12,22 @@ jobs:
steps:
# Launch workflow using Tower CLI tool action {%- raw %}
- name: Launch workflow via tower
uses: seqeralabs/action-tower-launch@v1
uses: seqeralabs/action-tower-launch@v2
with:
workspace_id: ${{ secrets.TOWER_WORKSPACE_ID }}
access_token: ${{ secrets.TOWER_ACCESS_TOKEN }}
compute_env: ${{ secrets.TOWER_COMPUTE_ENV }}
revision: ${{ github.sha }}
workdir: s3://${{ secrets.AWS_S3_BUCKET }}{% endraw %}/work/{{ short_name }}/{% raw %}work-${{ github.sha }}{% endraw %}
parameters: |
{
"outdir": "s3://{% raw %}${{ secrets.AWS_S3_BUCKET }}{% endraw %}/{{ short_name }}/{% raw %}results-test-${{ github.sha }}{% endraw %}"
}
profiles: test

- uses: actions/upload-artifact@v3
with:
name: Tower debug log file
path: tower_action_*.log
path: |
tower_action_*.log
tower_action_*.json
14 changes: 14 additions & 0 deletions tests/modules/lint.py
Original file line number Diff line number Diff line change
@@ -82,6 +82,20 @@ def test_modules_lint_multiple_remotes(self):
assert len(module_lint.warned) >= 0


def test_modules_lint_registry(self):
"""Test linting the samtools module and alternative registry"""
self.mods_install.install("samtools")
module_lint = nf_core.modules.ModuleLint(dir=self.pipeline_dir)
module_lint.lint(print_results=False, registry="public.ecr.aws", module="samtools")
assert len(module_lint.failed) == 0, f"Linting failed with {[x.__dict__ for x in module_lint.failed]}"
assert len(module_lint.passed) > 0
assert len(module_lint.warned) >= 0
module_lint.lint(print_results=False, registry="quay.io", module="samtools")
assert len(module_lint.failed) == 0, f"Linting failed with {[x.__dict__ for x in module_lint.failed]}"
assert len(module_lint.passed) > 0
assert len(module_lint.warned) >= 0


def test_modules_lint_patched_modules(self):
"""
Test creating a patch file and applying it to a new version of the the files

0 comments on commit 8c96eb4

Please sign in to comment.