Merge lp:~raharper/curtin/trunk.fix-tox-errors into lp:~curtin-dev/curtin/trunk

Proposed by Ryan Harper
Status: Merged
Merged at revision: 366
Proposed branch: lp:~raharper/curtin/trunk.fix-tox-errors
Merge into: lp:~curtin-dev/curtin/trunk
Diff against target: 56 lines (+7/-7)
2 files modified
curtin/commands/main.py (+5/-5)
tox.ini (+2/-2)
To merge this branch: bzr merge lp:~raharper/curtin/trunk.fix-tox-errors
Reviewer Review Type Date Requested Status
Server Team CI bot continuous-integration Approve
Scott Moser Pending
Review via email: mp+290531@code.launchpad.net

Description of the change

curtin: fix tox errors in commands/main.py

Unexpectedly, pylint now blows up with the following errors:

E:208,13: Instance of 'argv' has no 'reportstack' member (but some types could
      not be inferred) (maybe-no-member)
E:209,18: Instance of 'argv' has no 'func' member (but some types could not be
      inferred) (maybe-no-member)

This code has been in tree for a very long time, this only shows up when
we added pylint to the tox run (revno 335). This should have always failed.

With that said, the fix is simple. In main(), we define args to be sys.argv
but it's not used until later we re-assign args to be something else from
argparser. Removing that useless initialization as a argv type resolves
the lint error.

To post a comment you must log in.
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Scott Moser (smoser) wrote :

I've just verified that what changed was a change in behavior from pylint from 1.5.4 to 1.5.5 (which was uploaded 2016-03-21) https://pypi.python.org/pypi/pylint .

this is the crux of using tox.
Its obnoxious when some upload somewhere on the internet starts breaking your c-i.

The thing we could do is set the version of pylint that is installed explicitly.
That can be done with something like:
=== modified file 'tox.ini'
--- tox.ini 2016-03-18 14:05:02 +0000
+++ tox.ini 2016-03-30 23:43:34 +0000
@@ -38,7 +38,7 @@
 # set basepython because tox 1.6 (trusty) does not support generated environments
 basepython = python3
 deps = {[testenv]deps}
- pylint
+ pylint==1.5.4
     bzr+lp:simplestreams
 commands = {envpython} -m pylint --errors-only {posargs:curtin tests/vmtests}

Revision history for this message
Scott Moser (smoser) wrote :

i'm ok with the change you suggest though.
The one thing is that when this code was put in, the intent was probably to allow args to be passed into main directly without the user having to modify sys.argv, but the code was lacking in doing that.

I suspect a patch like this would be more along the lines of what i intended, it also passes pylint.

http://paste.ubuntu.com/15564028/

Revision history for this message
Ryan Harper (raharper) wrote :

I'll likely take both your changes here once I can get vmtest to sync
images properly to my system for testing.

On Wed, Mar 30, 2016 at 6:55 PM, Scott Moser <email address hidden> wrote:

> i'm ok with the change you suggest though.
> The one thing is that when this code was put in, the intent was probably
> to allow args to be passed into main directly without the user having to
> modify sys.argv, but the code was lacking in doing that.
>
> I suspect a patch like this would be more along the lines of what i
> intended, it also passes pylint.
>
> http://paste.ubuntu.com/15564028/
> --
>
> https://code.launchpad.net/~raharper/curtin/trunk.fix-tox-errors/+merge/290531
> You are the owner of lp:~raharper/curtin/trunk.fix-tox-errors.
>

367. By Ryan Harper

Further fixes: specify pylint version and restore use-case of handling args

Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'curtin/commands/main.py'
2--- curtin/commands/main.py 2016-01-07 16:21:29 +0000
3+++ curtin/commands/main.py 2016-03-31 00:55:53 +0000
4@@ -117,9 +117,9 @@
5 return
6
7
8-def main(args=None):
9- if args is None:
10- args = sys.argv
11+def main(argv=None):
12+ if argv is None:
13+ argv = sys.argv[1:]
14
15 stacktrace = (os.environ.get('CURTIN_STACKTRACE', "0").lower()
16 not in ("0", "false", ""))
17@@ -129,7 +129,7 @@
18 except ValueError:
19 verbosity = 1
20
21- maybe_install_deps(sys.argv[1:], stacktrace=stacktrace,
22+ maybe_install_deps(argv, stacktrace=stacktrace,
23 verbosity=verbosity)
24
25 # Above here, only standard library modules can be assumed.
26@@ -140,7 +140,7 @@
27 subps = parser.add_subparsers(dest="subcmd")
28 for subcmd in SUB_COMMAND_MODULES:
29 add_subcmd(subps, subcmd)
30- args = parser.parse_args(sys.argv[1:])
31+ args = parser.parse_args(argv)
32
33 # merge config flags into a single config dictionary
34 cfg_opts = args.main_cfgopts
35
36=== modified file 'tox.ini'
37--- tox.ini 2016-03-18 14:05:02 +0000
38+++ tox.ini 2016-03-31 00:55:53 +0000
39@@ -38,7 +38,7 @@
40 # set basepython because tox 1.6 (trusty) does not support generated environments
41 basepython = python3
42 deps = {[testenv]deps}
43- pylint
44+ pylint==1.5.4
45 bzr+lp:simplestreams
46 commands = {envpython} -m pylint --errors-only {posargs:curtin tests/vmtests}
47
48@@ -46,7 +46,7 @@
49 # set basepython because tox 1.6 (trusty) does not support generated environments
50 basepython = python2.7
51 deps = {[testenv]deps}
52- pylint
53+ pylint==1.5.4
54 commands = {envpython} -m pylint --errors-only {posargs:curtin}
55
56 [testenv:docs]

Subscribers

People subscribed via source and target branches