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

Experimental fast validator code path, using cwl-utils #1720

Merged
merged 31 commits into from
Sep 13, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
f6d8198
Experimental fast validator code path, using cwl-utils
tetron Aug 26, 2022
b145e3c
Load the schema graph
tetron Aug 27, 2022
3cb3d42
DO NOT MERGE: use cwl-utils from draft PR
mr-c Aug 31, 2022
a5a1b85
improve type hints
mr-c Aug 31, 2022
b00b17e
Fix load_tool typing
tetron Sep 1, 2022
cf846bc
Merge branch 'main' into pa/cwlutil-validator
tetron Sep 1, 2022
3adbcbc
GA: run the conformance tests with the fast validator
mr-c Sep 2, 2022
772a11c
conformance testing: use CWLTOOL_OPTIONS
mr-c Sep 2, 2022
d140151
CWLTOOL_OPTIONS: ignore an empty string
mr-c Sep 2, 2022
80271c4
DO NOT MERGE: container build needs our cwl-util branch
mr-c Sep 2, 2022
8c1c82d
Merge branch 'main' into pa/cwlutil-validator
tetron Sep 2, 2022
09d08d9
Merge branch 'main' into pa/cwlutil-validator
tetron Sep 9, 2022
3d12d6e
Rename --fast-validator to --fast-parser
tetron Sep 9, 2022
3617dd5
mypy & format
tetron Sep 9, 2022
d6226ec
Fix mypy-requirements
tetron Sep 9, 2022
b503d54
Fix github action
tetron Sep 9, 2022
b686e41
use ResolveType
mr-c Sep 9, 2022
833b0d6
Add section to README about --fast-parser
tetron Sep 9, 2022
d64be75
Fast parser expects comment data
tetron Sep 9, 2022
1113260
format
tetron Sep 9, 2022
efa5c44
Update document_loader index to match fast parser
tetron Sep 10, 2022
acaf81c
Fix format
tetron Sep 10, 2022
0584fe8
Make sure everything in $graph gets into the index
tetron Sep 11, 2022
46f53bc
Don't crash update if filename field is missing
tetron Sep 12, 2022
03b2f02
Add loadingContext.skip_resolve_all with note
tetron Sep 12, 2022
7630a51
Fix format
tetron Sep 12, 2022
db08a47
Update README.rst
tetron Sep 13, 2022
1e150ab
conformance tests: report CWLTOOL_OPTIONS
mr-c Sep 13, 2022
f3e14e1
CI: include extra options in coverge classname
mr-c Sep 13, 2022
746033b
Bump cwl-utils version requirement
tetron Sep 13, 2022
2aa44d0
conformance coverage: normalize paths
mr-c Sep 13, 2022
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
6 changes: 6 additions & 0 deletions .github/workflows/ci-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,11 @@ jobs:
matrix:
cwl-version: [v1.0, v1.1, v1.2]
container: [docker, singularity, podman]
extras: [""]
include:
- cwl-version: v1.2
container: docker
extras: "--fast-parser"

steps:
- uses: actions/checkout@v3
Expand All @@ -141,6 +146,7 @@ jobs:
version: ${{ matrix.cwl-version }}
container: ${{ matrix.container }}
spec_branch: main
CWLTOOL_OPTIONS: ${{ matrix.extras }}
run: ./conformance-test.sh

release_test:
Expand Down
13 changes: 13 additions & 0 deletions README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -667,6 +667,19 @@ given in the following table; all are optional.
+----------------+------------------+----------+------------------------------+


Enabling Fast Parser (experimental)
-----------------------------------

For very large workflows, `cwltool` can spend a lot of time in
initialization, before the first step runs. There is an experimental
flag ``--fast-parser`` which can dramatically reduce the
initialization overhead, however as of this writing it has several limitations:

- Error reporting in general is worse than the standard parser, you will want to use it with workflows that you know are already correct.

- It does not check for dangling links (these will become runtime errors instead of loading errors)

- Several other cases fail, as documented in https://github.com/common-workflow-language/cwltool/pull/1720

===========
Development
Expand Down
21 changes: 11 additions & 10 deletions conformance-test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ pip3 install -U setuptools wheel pip
pip3 uninstall -y cwltool
pip3 install -e .
pip3 install codecov cwltest>=2.1
root_folder=${PWD}
pushd "${repo}-${spec_branch}" || exit 1

# shellcheck disable=SC2043
Expand All @@ -71,6 +72,7 @@ cat > "${COVERAGE_RC}" <<EOF
[run]
branch = True
source_pkgs = cwltool
source = ${root_folder}

[report]
exclude_lines =
Expand All @@ -92,15 +94,15 @@ chmod a+x "${CWLTOOL_WITH_COV}"
unset exclusions
declare -a exclusions

EXTRA="--parallel"
CWLTOOL_OPTIONS+=" --parallel"
# shellcheck disable=SC2154
if [[ "$version" = *dev* ]]
then
EXTRA+=" --enable-dev"
CWLTOOL_OPTIONS+=" --enable-dev"
fi

if [[ "$container" = "singularity" ]]; then
EXTRA+=" --singularity"
CWLTOOL_OPTIONS+=" --singularity"
# This test fails because Singularity and Docker have
# different views on how to deal with this.
exclusions+=(docker_entrypoint)
Expand All @@ -113,13 +115,9 @@ if [[ "$container" = "singularity" ]]; then
exclusions+=(stdin_shorcut)
fi
elif [[ "$container" = "podman" ]]; then
EXTRA+=" --podman"
CWLTOOL_OPTIONS+=" --podman"
fi

if [ -n "$EXTRA" ]
then
EXTRA="EXTRA=${EXTRA}"
fi
if [ "$GIT_BRANCH" = "origin/main" ] && [[ "$version" = "v1.0" ]] && [[ "$container" = "docker" ]]
then
rm -Rf conformance
Expand All @@ -133,6 +131,7 @@ Conformance test of cwltool ${tool_ver} for CWL ${version}
Commit: ${GIT_COMMIT}
Python version: 3
Container: ${container}
Extra options: ${CWLTOOL_OPTIONS}
EOM
)

Expand All @@ -148,11 +147,13 @@ if (( "${#exclusions[*]}" > 0 )); then
else
EXCLUDE=""
fi
export CWLTOOL_OPTIONS
echo CWLTOOL_OPTIONS="${CWLTOOL_OPTIONS}"
# shellcheck disable=SC2086
LC_ALL=C.UTF-8 ./run_test.sh --junit-xml=result3.xml ${EXCLUDE} \
RUNNER=${CWLTOOL_WITH_COV} "-j$(nproc)" ${BADGE} \
${DRAFT} "${EXTRA}" \
"--classname=py3_${container}"
${DRAFT} \
"--classname=py3_${container}_$(echo ${CWLTOOL_OPTIONS} | tr "[:blank:]-" _)"
# LC_ALL=C is to work around junit-xml ASCII only bug

# capture return code of ./run_test.sh
Expand Down
7 changes: 7 additions & 0 deletions cwltool/argparser.py
Original file line number Diff line number Diff line change
Expand Up @@ -576,6 +576,13 @@ def arg_parser() -> argparse.ArgumentParser:
default=True,
help=argparse.SUPPRESS,
)
parser.add_argument(
"--fast-parser",
dest="fast_parser",
action="store_true",
default=False,
help=argparse.SUPPRESS,
)

reggroup = parser.add_mutually_exclusive_group()
reggroup.add_argument(
Expand Down
20 changes: 18 additions & 2 deletions cwltool/context.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,18 @@
import shutil
import tempfile
import threading
from typing import IO, Any, Callable, Dict, Iterable, List, Optional, TextIO, Union
from typing import (
IO,
Any,
Callable,
Dict,
Iterable,
List,
Optional,
TextIO,
Tuple,
Union,
)

# move to a regular typing import when Python 3.3-3.6 is no longer supported
from ruamel.yaml.comments import CommentedMap
Expand All @@ -23,6 +34,8 @@
from .utils import DEFAULT_TMP_PREFIX, CWLObjectType, HasReqsHints, ResolverType

if TYPE_CHECKING:
from cwl_utils.parser.cwl_v1_2 import LoadingOptions

from .process import Process
from .provenance import ResearchObject # pylint: disable=unused-import
from .provenance_profile import ProvenanceProfile
Expand Down Expand Up @@ -102,7 +115,10 @@ def __init__(self, kwargs: Optional[Dict[str, Any]] = None) -> None:
self.relax_path_checks = False # type: bool
self.singularity = False # type: bool
self.podman = False # type: bool
self.eval_timeout = 60 # type: float
self.eval_timeout: float = 60
self.codegen_idx: Dict[str, Tuple[Any, "LoadingOptions"]] = {}
self.fast_parser = False
self.skip_resolve_all = False

super().__init__(kwargs)

Expand Down
Loading