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

Fix verdi export create when specifying computers #1448

Merged

Conversation

sphuber
Copy link
Contributor

@sphuber sphuber commented Apr 19, 2018

Fixes #1033

Only passing computers to verdi export create was not supported by export and hence broken.
I added the necessary logic to make computers exportable.

Additionally, I fixed the parsing of the verdi export create command.
Click does support multiple values for an option through the multiple keyword
of the option constructor, however, this requires the user to repeat the flag
for each value, i.e.:

--nodes 1 --nodes 2 --nodes 15

For big lists, this is impractical and untenable. Also it is not very intuitive and users were using it the way below, which would throw cryptic errors. Therefore we introduce a custom
MultiValueOption that instead supports the following notation:

--nodes 1 2 15

Since these options are greedy, they can clash with cli commands that also take
arguments. If the greedy option is used, the argument will be mistaken for another
option value. In this case, the MultiValueOption respects the endopts marker '--'
that is the standard for POSIX command line interfaces. Anything after this marker
will be considered as arguments by the parser.

@sphuber sphuber requested review from DropD and giovannipizzi April 19, 2018 21:30
sphuber added 2 commits April 20, 2018 10:08
…ption

Click does support multiple values for an option through the `multiple` keyword
of the option constructor, however, this requires the user to repeat the flag
for each value, i.e.:

    --nodes 1 --nodes 2 --nodes 15

For big lists, this is impractical and untenable. Therefore we introduce a custom
MultiValueOption that instead supports the following notation:

    --nodes 1 2 15

Since these options are greedy, they can clash with cli commands that also take
arguments. If the greedy option is used, the argument will be mistaken for another
option value. In this case, the MultiValueOption respects the endopts marker '--'
that is the standard for POSIX command line interfaces. Anything after this marker
will be considered as arguments by the parser.
@sphuber sphuber force-pushed the fix_1033_verdi_export_create_computer branch from 9b26f63 to cb01893 Compare April 20, 2018 08:32
@codecov-io
Copy link

Codecov Report

Merging #1448 into workflows will increase coverage by 0.06%.
The diff coverage is 45.61%.

Impacted file tree graph

@@              Coverage Diff              @@
##           workflows    #1448      +/-   ##
=============================================
+ Coverage      56.45%   56.51%   +0.06%     
=============================================
  Files            270      270              
  Lines          33517    33568      +51     
=============================================
+ Hits           18922    18971      +49     
- Misses         14595    14597       +2
Impacted Files Coverage Δ
aiida/utils/cli/options.py 46.37% <37.5%> (+46.37%) ⬆️
aiida/orm/importexport.py 78.98% <54.54%> (-0.3%) ⬇️
aiida/cmdline/commands/exportfile.py 11.36% <83.33%> (+0.33%) ⬆️
aiida/utils/cli/validators.py 10% <0%> (+10%) ⬆️
aiida/utils/cli/__init__.py 37.5% <0%> (+37.5%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 762b1ab...cb01893. Read the comment docs.

@sphuber sphuber merged commit 328d1ac into aiidateam:workflows Apr 20, 2018
@sphuber sphuber deleted the fix_1033_verdi_export_create_computer branch April 20, 2018 08:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants