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

check-software: update to accept module arguments #2769

Merged

Conversation

sadielbartholomew
Copy link
Collaborator

@sadielbartholomew sadielbartholomew commented Sep 4, 2018

As requested in #2734 (comment), return a zero or non-zero output for a software dependency pass or fail, respectively. cylc check-software will under this implementation accept as an argument any module listed in the specification (the keys under opt_spec, including the lower-case form). Without arguments it behaves as before.

@sadielbartholomew sadielbartholomew added this to the soon milestone Sep 4, 2018
@sadielbartholomew sadielbartholomew self-assigned this Sep 4, 2018
@sadielbartholomew
Copy link
Collaborator Author

sadielbartholomew commented Sep 4, 2018

I still need to check if the documentation & CLI help option (etc.) need updating for this. I will address that shortly. --help output from docstring & User Guide updated accordingly.

@TomekTrzeciak
Copy link
Contributor

Thanks for not forgetting about this 👍

else check for all package dependencies and print the results in a table.
"""
# Check for valid module argument; if present return 0 or 1 for pass/fail.
if len(sys.argv) > 1:
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we check for multiple dependencies with a for loop here? Also, I think it would be nicer to signal success/failure through exit code rather than STDOUT. How about the following?

    exit_code = 0
    for cmd_arg in sys.argv[1:]:
        mod_arg = upper_case_conv.get(cmd_arg.lower(), cmd_arg.lower())
        if mod_arg not in module_args:
            sys.stderr.write('No such module in the software dependencies: ' +
                             cmd_arg + '\n')
            exit_code = max(exit_code, 2)
        else:
            exit_code = max(exit_code, individual_status_print(mod_arg))
    if len(sys.argv) > 1:
        sys.exit(exit_code)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, sorry, an exit code would be more appropriate. Okay, we can accept multiple dependency arguments too.

@sadielbartholomew
Copy link
Collaborator Author

sadielbartholomew commented Sep 5, 2018

I've just realised a docstring and the --help are not quite right as non-zero exit code is an integer giving the number of failed modules. Will rectify soon (currently preparing for imminent training, so not quite now). Everything up-to-date now.

@sadielbartholomew sadielbartholomew force-pushed the check-software-update branch 2 times, most recently from 7527e76 to 74fc6be Compare September 7, 2018 15:30
@matthewrmshin matthewrmshin modified the milestones: soon, next release Sep 13, 2018
Copy link
Member

@hjoliver hjoliver left a comment

Choose a reason for hiding this comment

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

One minor typo: in the first line of command help [ARGS] should be [MODULES] (and no need for ... as plural already).

@sadielbartholomew
Copy link
Collaborator Author

Well spotted @hjoliver! I've rectified that in a new commit & squashed it into the original.

@oliver-sanders oliver-sanders merged commit 3e41da2 into cylc:master Oct 1, 2018
@sadielbartholomew sadielbartholomew deleted the check-software-update branch October 9, 2018 15:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants