Skip to content

implement basic support for type checking of easyconfig parameters#1427

Merged
boegel merged 7 commits intoeasybuilders:developfrom
boegel:easyconfig_param_type_checking
Oct 14, 2015
Merged

implement basic support for type checking of easyconfig parameters#1427
boegel merged 7 commits intoeasybuilders:developfrom
boegel:easyconfig_param_type_checking

Conversation

@boegel
Copy link
Copy Markdown
Member

@boegel boegel commented Oct 14, 2015

Without type checking, you can run into nasty errors, for example the one below when using a non-string value for the version easyconfig parameter:

Traceback (most recent call last):
  File "/Users/kehoste/work/easybuild-framework/easybuild/main.py", line 361, in <module>
    main()
  File "/Users/kehoste/work/easybuild-framework/easybuild/main.py", line 276, in main
    easyconfigs, generated_ecs = parse_easyconfigs(paths)
  File "/Users/kehoste/work/easybuild-framework/easybuild/framework/easyconfig/tools.py", line 345, in parse_easyconfigs
    ecs = process_easyconfig(ec_file, **kwargs)
  File "/Users/kehoste/work/easybuild-framework/easybuild/framework/easyconfig/easyconfig.py", line 943, in process_easyconfig
    ec = EasyConfig(spec, build_specs=build_specs, validate=validate, hidden=hidden)
  File "/Users/kehoste/work/easybuild-framework/easybuild/framework/easyconfig/easyconfig.py", line 188, in __init__
    self.parse()
  File "/Users/kehoste/work/easybuild-framework/easybuild/framework/easyconfig/easyconfig.py", line 296, in parse
    self.generate_template_values()
  File "/Users/kehoste/work/easybuild-framework/easybuild/framework/easyconfig/easyconfig.py", line 661, in generate_template_values
    self._generate_template_values(skip_lower=True)
  File "/Users/kehoste/work/easybuild-framework/easybuild/framework/easyconfig/easyconfig.py", line 675, in _generate_template_values
    ignore=ignore, skip_lower=skip_lower)
  File "/Users/kehoste/work/easybuild-framework/easybuild/framework/easyconfig/templates.py", line 161, in template_constant_dict
    version = LooseVersion(version).version
  File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/distutils/version.py", line 265, in __init__
    self.parse(vstring)
  File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/distutils/version.py", line 274, in parse
    self.component_re.split(vstring))
TypeError: expected string or buffer

With these changes in place, you'll get this instead:

EasyBuildError: "Failed to process easyconfig example.eb: Type checking of easyconfig parameter values failed: value for 'version' should be of type 'basestring'"

This implementation is intended as a starting point, since only the values for name/version will be type-checked for now.
Complex values like sanity_check_paths will need additional logic/functionality, and support for automatically trying to convert values is probably worthwhile too. For example, with easyconfig files in YAML syntax (see #1407), a value for version like 1.6 would be parsed as float by default, although it should be a string value...

@boegel boegel added this to the v2.4.0 milestone Oct 14, 2015
@boegel
Copy link
Copy Markdown
Member Author

boegel commented Oct 14, 2015

@wpoely86, @Caylo: please review?

wpoely86
wpoely86 reviewed Oct 14, 2015
View reviewed changes
Comment thread easybuild/framework/easyconfig/parser.py Outdated
Copy link
Copy Markdown
Member

@wpoely86 wpoely86 Oct 14, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

document args

@wpoely86
Copy link
Copy Markdown
Member

wpoely86 commented Oct 14, 2015

lgtm

@hpcugentbot
Copy link
Copy Markdown

hpcugentbot commented Oct 14, 2015

EasyBuild framework unit test suite PASSed (see https://jenkins1.ugent.be/job/easybuild-framework-pr-builder/2196/console for more details).

This pull request is now ready for review/testing.

Please try and find someone who can tackle this; contact @boegel if you're not sure what to do.

@hpcugentbot
Copy link
Copy Markdown

hpcugentbot commented Oct 14, 2015

EasyBuild framework unit test suite PASSed (see https://jenkins1.ugent.be/job/easybuild-framework-pr-builder/2197/console for more details).

This pull request is now ready for review/testing.

Please try and find someone who can tackle this; contact @boegel if you're not sure what to do.

@boegel
Copy link
Copy Markdown
Member Author

boegel commented Oct 14, 2015

Thanks for the review @wpoely86 and @Caylo!

boegel added a commit that referenced this pull request Oct 14, 2015
@boegel
Merge pull request #1427 from boegel/easyconfig_param_type_checking
dfe5547 
implement basic support for type checking of easyconfig parameters
@boegel boegel merged commit dfe5547 into easybuilders:develop Oct 14, 2015
@boegel boegel deleted the easyconfig_param_type_checking branch October 14, 2015 11:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

v2.4.0

Development

Successfully merging this pull request may close these issues.

3 participants

@boegel @wpoely86 @hpcugentbot