-
Notifications
You must be signed in to change notification settings - Fork 47
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
global: allow operational options in reana.yaml #400
Merged
tiborsimko
merged 5 commits into
reanahub:master
from
mvidalgarcia:356-toplevel-configurable
Apr 8, 2020
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
b9875a6
global: allow passing options to workflow loader
mvidalgarcia bd0723d
global: plug options validation from reana-commons
mvidalgarcia 8f215c8
global: validate reana.yaml operational options
mvidalgarcia 377ae0a
yadage: prevent loading workflow spec on creation
mvidalgarcia e2eb481
cwl: switch subprocess run method
mvidalgarcia File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,6 +19,8 @@ | |
import yadageschemas | ||
import yaml | ||
from jsonschema import ValidationError, validate | ||
from reana_commons.errors import REANAValidationError | ||
from reana_commons.operational_options import validate_operational_options | ||
from reana_commons.serial import serial_load | ||
from reana_commons.utils import get_workflow_status_change_verb | ||
|
||
|
@@ -38,7 +40,7 @@ def workflow_uuid_or_name(ctx, param, value): | |
return value | ||
|
||
|
||
def yadage_load(workflow_file, toplevel='.'): | ||
def yadage_load(workflow_file, toplevel='.', **kwargs): | ||
"""Validate and return yadage workflow specification. | ||
|
||
:param workflow_file: A specification file compliant with | ||
|
@@ -65,37 +67,28 @@ def yadage_load(workflow_file, toplevel='.'): | |
} | ||
|
||
try: | ||
return yadageschemas.load(spec=workflow_file, specopts=specopts, | ||
validopts=validopts, validate=True) | ||
yadageschemas.load(spec=workflow_file, specopts=specopts, | ||
validopts=validopts, validate=True) | ||
|
||
except ValidationError as e: | ||
e.message = str(e) | ||
raise e | ||
|
||
|
||
def cwl_load(workflow_file): | ||
def cwl_load(workflow_file, **kwargs): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here. For CWL workflows, when passing |
||
"""Validate and return cwl workflow specification. | ||
|
||
:param workflow_file: A specification file compliant with | ||
`cwl` workflow specification. | ||
:returns: A dictionary which represents the valid `cwl` workflow. | ||
""" | ||
result = \ | ||
subprocess.run( | ||
['cwltool', '--pack', '--quiet', workflow_file], | ||
stdout=subprocess.PIPE) | ||
value = result.stdout.decode('utf-8') | ||
subprocess.check_output( | ||
['cwltool', '--pack', '--quiet', workflow_file]) | ||
value = result.decode('utf-8') | ||
return json.loads(value) | ||
|
||
|
||
workflow_load = { | ||
'yadage': yadage_load, | ||
'cwl': cwl_load, | ||
'serial': serial_load, | ||
} | ||
"""Dictionary to extend with new workflow specification loaders.""" | ||
|
||
|
||
def load_workflow_spec(workflow_type, workflow_file, **kwargs): | ||
"""Validate and return machine readable workflow specifications. | ||
|
||
|
@@ -104,16 +97,44 @@ def load_workflow_spec(workflow_type, workflow_file, **kwargs): | |
specification. | ||
:returns: A dictionary which represents the valid workflow specification. | ||
""" | ||
workflow_load = { | ||
'yadage': yadage_load, | ||
'cwl': cwl_load, | ||
'serial': serial_load, | ||
} | ||
"""Dictionary to extend with new workflow specification loaders.""" | ||
|
||
return workflow_load[workflow_type](workflow_file, **kwargs) | ||
|
||
|
||
def load_reana_spec(filepath, skip_validation=False): | ||
"""Load and validate reana specification file. | ||
|
||
:raises IOError: Error while reading REANA spec file from given filepath`. | ||
:raises IOError: Error while reading REANA spec file from given `filepath`. | ||
:raises ValidationError: Given REANA spec file does not validate against | ||
REANA specification. | ||
""" | ||
def _prepare_kwargs(reana_yaml): | ||
kwargs = {} | ||
workflow_type = reana_yaml['workflow']['type'] | ||
if workflow_type == 'serial': | ||
kwargs['specification'] = reana_yaml['workflow'].\ | ||
get('specification') | ||
kwargs['parameters'] = \ | ||
reana_yaml.get('inputs', {}).get('parameters', {}) | ||
kwargs['original'] = True | ||
|
||
if 'options' in reana_yaml['inputs']: | ||
try: | ||
reana_yaml['inputs']['options'] = validate_operational_options( | ||
workflow_type, | ||
reana_yaml['inputs']['options']) | ||
except REANAValidationError as e: | ||
click.secho(e.message, err=True, fg='red') | ||
sys.exit(1) | ||
kwargs.update(reana_yaml['inputs']['options']) | ||
return kwargs | ||
|
||
try: | ||
with open(filepath) as f: | ||
reana_yaml = yaml.load(f.read(), Loader=yaml.FullLoader) | ||
|
@@ -123,21 +144,14 @@ def load_reana_spec(filepath, skip_validation=False): | |
.format(filepath=filepath)) | ||
_validate_reana_yaml(reana_yaml) | ||
|
||
kwargs = {} | ||
if reana_yaml['workflow']['type'] == 'serial': | ||
kwargs['specification'] = reana_yaml['workflow'].\ | ||
get('specification') | ||
kwargs['parameters'] = \ | ||
reana_yaml.get('inputs', {}).get('parameters', {}) | ||
kwargs['original'] = True | ||
|
||
workflow_type = reana_yaml['workflow']['type'] | ||
reana_yaml['workflow']['specification'] = load_workflow_spec( | ||
reana_yaml['workflow']['type'], | ||
workflow_type, | ||
reana_yaml['workflow'].get('file'), | ||
**kwargs | ||
**_prepare_kwargs(reana_yaml) | ||
) | ||
|
||
if reana_yaml['workflow']['type'] == 'cwl' and \ | ||
if workflow_type == 'cwl' and \ | ||
'inputs' in reana_yaml: | ||
with open(reana_yaml['inputs']['parameters']['input']) as f: | ||
reana_yaml['inputs']['parameters'] = \ | ||
|
@@ -224,32 +238,27 @@ def get_workflow_name_and_run_number(workflow_name): | |
return workflow_name, '' | ||
|
||
|
||
def validate_cwl_operational_options(operational_options): | ||
"""Validate cwl operational options.""" | ||
forbidden_args = ['--debug', '--tmpdir-prefix', '--tmp-outdir-prefix' | ||
'--default-container', '--outdir'] | ||
for option in operational_options: | ||
if option in forbidden_args: | ||
click.echo('Operational option {0} are not allowed. \n' | ||
.format(operational_options), err=True) | ||
sys.exit(1) | ||
cmd = 'cwltool --version {0}'.format(' '.join(operational_options)) | ||
try: | ||
subprocess.check_call(cmd, shell=True) | ||
except subprocess.CalledProcessError as err: | ||
click.echo('Operational options {0} are not valid. \n' | ||
'{1}'.format(operational_options, err), err=True) | ||
sys.exit(1) | ||
|
||
|
||
def validate_serial_operational_options(operational_options): | ||
"""Return validated serial operational options.""" | ||
try: | ||
return dict(p.split('=') for p in operational_options) | ||
except Exception as err: | ||
click.echo('Operational options {0} are not valid. \n' | ||
'{1}'.format(operational_options, err), err=True) | ||
sys.exit(1) | ||
def get_workflow_root(): | ||
"""Return the current workflow root directory.""" | ||
reana_yaml = get_reana_yaml_file_path() | ||
workflow_root = os.getcwd() | ||
while True: | ||
file_list = os.listdir(workflow_root) | ||
parent_dir = os.path.dirname(workflow_root) | ||
if reana_yaml in file_list: | ||
break | ||
else: | ||
if workflow_root == parent_dir: | ||
click.echo(click.style( | ||
'Not a workflow directory (or any of the parent' | ||
' directories).\nPlease upload from inside' | ||
' the directory containing the reana.yaml ' | ||
'file of your workflow.', fg='red')) | ||
sys.exit(1) | ||
else: | ||
workflow_root = parent_dir | ||
workflow_root += '/' | ||
return workflow_root | ||
|
||
|
||
def validate_input_parameters(live_parameters, original_parameters): | ||
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Who is consuming these
**kwargs
? We already unpacktoplevel
which is the only one we are interested in.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we set other
inputs.options
such asinitdir
, its value will get here and this function would fail. With**kwargs
we can filter out these options that are not needed on validation time. I agree it's a bit hacky but it's cleanest and safest way I could come up with. If you have any other idea please let me know 🙂