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 argument parsing in python 2.7.9 #121

Closed
wants to merge 3 commits into from
Closed

Fix argument parsing in python 2.7.9 #121

wants to merge 3 commits into from

Conversation

Changaco
Copy link

With python 2.7.9 honcho doesn't read the env files provided via the -e command line argument, because of a change in the argparse module.

@Changaco
Copy link
Author

The change in the argparse module:

--- argparse-2.7.8.py   2014-12-18 18:44:37.956126224 +0100
+++ argparse-2.7.9.py   2014-12-18 18:44:23.426106089 +0100
@@ -1089,7 +1089,14 @@
         # parse all the remaining options into the namespace
         # store any unrecognized options on the object, so that the top
         # level parser can decide what to do with them
-        namespace, arg_strings = parser.parse_known_args(arg_strings, namespace)
+
+        # In case this subparser defines new defaults, we parse them
+        # in a new namespace object and then update the original
+        # namespace for the relevant parts.
+        subnamespace, arg_strings = parser.parse_known_args(arg_strings, None)
+        for key, value in vars(subnamespace).items():
+            setattr(namespace, key, value)
+
         if arg_strings:
             vars(namespace).setdefault(_UNRECOGNIZED_ARGS_ATTR, [])
             getattr(namespace, _UNRECOGNIZED_ARGS_ATTR).extend(arg_strings)

@slafs
Copy link
Collaborator

slafs commented Dec 19, 2014

Thanks for bringing this up.
I've restarted our latest master build on Travis-CI and yeah... now the build on python 2.7.9 is failing.

I'm not very familiar with argparse to be honest, but it seems that your PR fixes those tests that are broken on 2.7.9 now. Although it introduced some problems with test_export.py

@slafs slafs mentioned this pull request Dec 19, 2014
@Changaco
Copy link
Author

I've found a backward-compatible way of fixing the problem, tests are now passing.

@slafs
Copy link
Collaborator

slafs commented Dec 20, 2014

Thanks but now I think it's even more broken than before. A simple honcho --help gives me

usage: honcho [-e ENV] [-d APP_ROOT] [-f PROCFILE] [-v]
              {check,export,help,run,start,version} ...
honcho: error: too few arguments

on both python 2.7 (including 2.7.9) and 3.4.

I will have to take a closer look on this I guess.

@Changaco
Copy link
Author

Problem fixed by removing add_help=False.

erikbern pushed a commit to spotify/luigi that referenced this pull request Feb 2, 2015
Found some claims that a regression was introduced in Python 2.7.9 that might cause issues:
- nickstenning/honcho#121
- http://code.activestate.com/lists/python-dev/134014/
@nickstenning
Copy link
Owner

Thank you for your help. I've reworked this slightly and committed it as 1319da7. It's now released as v0.6.4.

thusoy added a commit to thusoy/public-pillar that referenced this pull request May 22, 2015
Ref. http://bugs.python.org/issue23058, this didn't seem to be intended to
be supported, so let's do it another way. Creds to
nickstenning/honcho#121 for workaround.
@ecdsa
Copy link

ecdsa commented Feb 22, 2017

Thank you for your help. I have had a related issue in electrum.
As a workaround, I reverted the last change to argparse._SubParsersAction.__call__
see spesmilo/electrum@1d1d76b

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.

4 participants