Skip to content
This repository has been archived by the owner on Jan 19, 2018. It is now read-only.

Fixes mode argument with --mode=genanswers #793

Merged
merged 1 commit into from
Aug 30, 2016

Conversation

cdrage
Copy link
Member

@cdrage cdrage commented Aug 30, 2016

When sudo atomic run projectatomic/helloapache --mode=genanswers is
supplied, the resulting error shows that --mode=genanswers is an
unrecognized argument.

This was introduced after refactoring the cli/main.py code to NOT include
uneeded parameters such as --provider-auth to CLI options such as init
and genanswers.

By adding self.parser.parse_known_args, this will ignore the added
--mode=genaswers to the CLI.

Fixes #792

@coveralls
Copy link

Coverage Status

Coverage remained the same at 64.302% when pulling 39c3565 on cdrage:master into 0ebf267 on projectatomic:master.

@@ -483,7 +483,7 @@ def run(self):
cmdline.insert(0, args.action) # Place 'action' at front

# Finally, parse args and give error if necessary
args = self.parser.parse_args(cmdline)
args, unknown = self.parser.parse_known_args(cmdline)
Copy link
Contributor

Choose a reason for hiding this comment

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

instead of doing this why don't we just delete the --mode arg from args on line 482

@coveralls
Copy link

Coverage Status

Coverage remained the same at 64.302% when pulling 30ab3b4 on cdrage:master into 0ebf267 on projectatomic:master.

@dustymabe
Copy link
Contributor

without actually testing this out, i'll say that the code change LGTM. Charlie, do you mind running through some manual testing before you merge?

@coveralls
Copy link

Coverage Status

Coverage remained the same at 64.302% when pulling 2e184ac on cdrage:master into 0ebf267 on projectatomic:master.

@cdrage
Copy link
Member Author

cdrage commented Aug 30, 2016

#dotests

@cdrage
Copy link
Member Author

cdrage commented Aug 30, 2016

@dustymabe sure, going to use init and genanswers to test this.

@cdrage
Copy link
Member Author

cdrage commented Aug 30, 2016

@dustymabe init doesn't work, genanswers works fine.

INFO   :: - cli/main.py :: Atomic App: 0.6.2 - Mode: Init
DEBUG  :: - cli/main.py :: Final parsed cmdline: init projectatomic/helloapache -v --mode=init
ERROR  :: - cli/main.py :: [Errno 2] No such file or directory: '/tmp/nulecule-new-app-9rirsK/artifacts/docker/projectatomic/helloapache_run'
Traceback (most recent call last):
  File "/usr/local/lib/python2.7/dist-packages/atomicapp-0.6.2-py2.7.egg/atomicapp/cli/main.py", line 106, in cli_init
    argdict['destination'])
  File "/usr/local/lib/python2.7/dist-packages/atomicapp-0.6.2-py2.7.egg/atomicapp/nulecule/main.py", line 183, in init
    with open(file_path, 'w') as f:
IOError: [Errno 2] No such file or directory: '/tmp/nulecule-new-app-9rirsK/artifacts/docker/projectatomic/helloapache_run'

is it because when using --mode genanswers will ignore the first argument? (in this case projectatomic/helloapache) and init does not?

@cdrage
Copy link
Member Author

cdrage commented Aug 30, 2016

#dotests

When `sudo atomic run projectatomic/helloapache --mode=genanswers` is
supplied, the resulting error shows that `--mode=genanswers` is an
unrecognized argument.

This was introduced after refactoring the cli/main.py code to NOT include
uneeded parameters such as --provider-auth to CLI options such as init
and genanswers.

By moving the --mode parser to the globals_parser, each command will
have the --mode option available.

Fixes projectatomic#792
@coveralls
Copy link

Coverage Status

Coverage remained the same at 64.302% when pulling af728b3 on cdrage:master into 0ebf267 on projectatomic:master.

@cdrage
Copy link
Member Author

cdrage commented Aug 30, 2016

OpenShift timed out ^^ ran locally and tests pass. Mergin'

@cdrage cdrage merged commit e9fa288 into projectatomic:master Aug 30, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants