Skip to content

Print usage when no args provided#404

Merged
FrankD412 merged 1 commit intollnl:developfrom
kinow:print-usage
Jun 29, 2022
Merged

Print usage when no args provided#404
FrankD412 merged 1 commit intollnl:developfrom
kinow:print-usage

Conversation

@kinow
Copy link
Copy Markdown
Contributor

@kinow kinow commented Jun 28, 2022 •
edited
Loading

Hi,

Running maestro for the first time ever today from the develop branch. I first tried maestro without anything else, but it resulted in a traceback.

(venv) kinow@ranma:~/Development/python/workspace/maestrowf$ maestro 
Traceback (most recent call last):
  File "/home/kinow/Development/python/workspace/maestrowf/venv/bin/maestro", line 8, in <module>
    sys.exit(main())
  File "/home/kinow/Development/python/workspace/maestrowf/maestrowf/maestro.py", line 495, in main
    if not hastattr(args, 'func'):
NameError: name 'hastattr' is not defined

I think it would be best to print the program usage instead? So if users are not aware, or not sure, which subcommand/subparser to trigger, they can read the usage and decide then.

Thanks for maestro! 👋
-Bruno

EDIT: closes #188 (hadn't seen the issue, sorry)

@FrankD412
Copy link
Copy Markdown
Member

FrankD412 commented Jun 28, 2022

Hi @kinow -- Thanks for the contribution! Must say you have impeccable timing; I had someone encounter this recently, so it was on my mind.

I've approved the MR to run through our CI and will review the code in a bit. :)

Thanks again for your contribution!

@FrankD412 FrankD412 self-requested a review June 28, 2022 17:49
@FrankD412 FrankD412 added Confirmed Confirmed reproduction of a posted bug. Bugfix Discussion/Implementation of a solution to a bug (usually a PR) labels Jun 28, 2022
@FrankD412 FrankD412 added this to the Release 1.1.10 milestone Jun 28, 2022
FrankD412
FrankD412 requested changes Jun 29, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@FrankD412 FrankD412 left a comment

Choose a reason for hiding this comment

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

@kinow -- it looks like you can set the default function. I generally avoid hasattr since that's been considered bad practice from what I've seen. Talking with @jwhite242 and with some searching, it seems that this might be the better bet:

    parser = ArgumentParser(
        prog="maestro",
        description="The Maestro Workflow Conductor for specifying, launching"
        ", and managing general workflows.",
        formatter_class=RawTextHelpFormatter)
    # This call applies a default function to the main parser as described
    # here: https://stackoverflow.com/a/61680800
    parser.set_defaults(func=lambda args: parser.print_help())

@kinow
Copy link
Copy Markdown
Contributor Author

kinow commented Jun 29, 2022

Hi @FrankD412

I normally use hasattr or wrap it in a try-except block for a AttributeError (latter is normally faster IIRC?). But I have no preference, it was just the first idea that came to my mind.

+1 to using a default value if that's available. Give me 10 mins and I'll update the branch and test locally…

kinow
kinow commented Jun 29, 2022
View reviewed changes
Comment thread maestrowf/maestro.py
@FrankD412 FrankD412 self-requested a review June 29, 2022 04:15
@FrankD412 FrankD412 merged commit 06fd2f2 into llnl:develop Jun 29, 2022
@kinow kinow deleted the print-usage branch June 29, 2022 04:19
jwhite242 added a commit that referenced this pull request Dec 12, 2023
@jwhite242 @FrankD412
@dependabot @kinow @doutriaux1 @grosa1 @bgunnar5
1.1.10 Release (#432)
141eaaa 
1.1.10 Release (#432)

* Sync up read the docs config with dev environments using poetry (#399)
* Print usage on command line when no args are provided (#404)
* Add sacct fallback to slurm adapter to improve robustness of job tracking (#405)
* Update Flurm Job State mappings for flux versions >= 0.26 (#407)
* Bump certifi from 2021.10.8 to 2022.12.7 to address security issue (#409)
* Bump cryptography from 37.0.1 to 38.0.3 to address security issue (#410)
* Add missing shbang in unscheduled scripts from lsf adapter (#411)
* Update poetry lockfile to address dependabot flagged security issues (#412)
* Fix for Dockerfile smell DL3006 (#418)
* Port Maestro documentation to mkdocs and expand coverage of features and tutorials (#403)
* Update version info to be driven from pyproject.toml exclusively, and hook up to command line (#419)
* Pin mermaid to < 10.x due to api change (#422)
* Bump lock file certifi from 2022.12.7 to 2023.7.22 to address security issue (#426)
* Refactor flux adapter to avoid using pickle to talk to flux brokers installed in external environments (#415)
   Also adds flux integration tests to exercise against real flux brokers
* Add pager functionality to status command (#420)
* Patch broken flux job cancellation (#428)
* Insulate slurm adapters from user customization of squeue and sacct output formats (#431)
   Also adds live unit and integration tests for slurm adapter

---------

Co-authored-by: Francesco Di Natale <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Bruno P. Kinoshita <[email protected]>
Co-authored-by: Charles Doutriaux <[email protected]>
Co-authored-by: Giovanni Rosa <[email protected]>
Co-authored-by: Brian Gunnarson <[email protected]>
jwhite242 added a commit that referenced this pull request Feb 6, 2024
@jwhite242 @FrankD412
@dependabot @kinow @doutriaux1 @grosa1 @bgunnar5
Merge pull request #437 from LLNL/repo_reset/release/1.1.10
6fa62f1 
1.1.10 Release (#432)

* Sync up read the docs config with dev environments using poetry (#399)
* Print usage on command line when no args are provided (#404)
* Add sacct fallback to slurm adapter to improve robustness of job tracking (#405)
* Update Flurm Job State mappings for flux versions >= 0.26 (#407)
* Bump certifi from 2021.10.8 to 2022.12.7 to address security issue (#409)
* Bump cryptography from 37.0.1 to 38.0.3 to address security issue (#410)
* Add missing shbang in unscheduled scripts from lsf adapter (#411)
* Update poetry lockfile to address dependabot flagged security issues (#412)
* Fix for Dockerfile smell DL3006 (#418)
* Port Maestro documentation to mkdocs and expand coverage of features and tutorials (#403)
* Update version info to be driven from pyproject.toml exclusively, and hook up to command line (#419)
* Pin mermaid to < 10.x due to api change (#422)
* Bump lock file certifi from 2022.12.7 to 2023.7.22 to address security issue (#426)
* Refactor flux adapter to avoid using pickle to talk to flux brokers installed in external environments (#415)
   Also adds flux integration tests to exercise against real flux brokers
* Add pager functionality to status command (#420)
* Patch broken flux job cancellation (#428)
* Insulate slurm adapters from user customization of squeue and sacct output formats (#431)
   Also adds live unit and integration tests for slurm adapter

---------

Co-authored-by: Francesco Di Natale <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Bruno P. Kinoshita <[email protected]>
Co-authored-by: Charles Doutriaux <[email protected]>
Co-authored-by: Giovanni Rosa <[email protected]>
Co-authored-by: Brian Gunnarson <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@FrankD412 FrankD412 FrankD412 approved these changes

Assignees

@kinow kinow

Labels

Bugfix Discussion/Implementation of a solution to a bug (usually a PR) Confirmed Confirmed reproduction of a posted bug.

Projects

None yet

Milestone

Release 1.1.10

Development

Successfully merging this pull request may close these issues.

AttributeError: 'Namespace' object has no attribute 'func'

2 participants

@kinow @FrankD412