Skip to content

Fix argument parsing in python 2.7.9#121

Closed
Changaco wants to merge 3 commits into
nickstenning:masterfrom
Changaco:master
Closed

Fix argument parsing in python 2.7.9#121
Changaco wants to merge 3 commits into
nickstenning:masterfrom
Changaco:master

Conversation

@Changaco

Copy link
Copy Markdown

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
Copy Markdown
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

slafs commented Dec 19, 2014

Copy link
Copy Markdown
Collaborator

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
Copy Markdown
Author

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

@slafs

slafs commented Dec 20, 2014

Copy link
Copy Markdown
Collaborator

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
Copy Markdown
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
Copy Markdown
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

ecdsa commented Feb 22, 2017

Copy link
Copy Markdown

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