add pylint to tox

[mbohlool]

following up on @marun's comment on #50:

Consider adding a linter check (e.g. flake8/pylint) to the tox configuration to catch these issues automatically.

[SEJeff]

@mbohlool I'm happy taking this on and have started doing it, however I was curious about all of the autogenerated code. Since much of this is autogenerated, how do we want to fix this? I'd rather not waste time on a bunch of fixup commits that are then blown away the next time things are stepped after an autogeneration is complete.

One approach is for me to fix swagger's codegen for python which is doable, but I really was never a java fan and don't relish the thought of installing and dealing with a JDK.

[SEJeff]

I'd also like to fix the requirements and test-requirements to look like every other file out there and not include spaces (ie: the sorted output from pip freeze more or less, but with the >=, etc version specifiers).

[mbohlool]

Manual changes to generated code is out of question as you argued. If there is a way to automatically fix some of those problems (look at scripts/update-pep8.sh), we should do it (and we can repeat it after each code generation) otherwise we should just ignore kubernetes.client package in the pylint checks.

[marun]

I'd recommend flake8 over pylint. And it's entirely possible to exclude files and paths from linting. I'm assuming generated code is cleanly separated from non-generated code? Here's an example of configuring flake8 in tox.ini:

https://github.com/openstack/neutron/blob/master/tox.ini#L132

[SEJeff]

@marun Yeah you just need a [flake8] section in the tox config, it isn't hard. I just wanted to flesh out the approach here with approval before spending time doing it. I'm going to implement it in this branch.

@mbohlool funnily enough, I've got some mad sed skills (a career mostly as a sysadmin will do that for you) and was 1/2 tempted to write a script to remove some of the obvious silliness in that autogenerated code. I'll have a go at that first and we can see if it is sensible. Also, do you have thoughts on adding coverage? I'd like to see coverage stats added if possible to this when we run tests. Seeing all of the noop tests that don't do anything does actually make me a bit sad.

I'd planned on doing it just like we do in home assistant, another OSS project I contribute to.

[marun]

I see value in automating linting of non-generated code, but I don't see the point of doing the same for generated code. If linting reveals functional problems with the generated code, the place to fix it is in the generator. Scripted cleanup can't help but be brittle in the face of generator changes.

[mbohlool]

Agreed with @marun, no value in linting generated code. I also sent a PR for coverage of only non-generated code #54

[SEJeff]

@mbohlool Cool, I can update my branch with those changes (excluding tests and kubernetes/client) and send it up tonight or tomorrow. I also added the py35 environment since my workstation distro, Fedora 25, lacks python 3.4, but default installs 3.5

[mbohlool]

@SEJeff use pyenv to create as much environment as you like and activate 3.4 and 2.7 for this repo:

pip install pyenv
pyenv install 2.7.12
pyenv install 3.4.3
cd client-python
pyenv local 2.7.12 3.4.3

You may need to restart your shell for it to take effect but tox should pick up these two newly installed version. I've never tested the whole client with 3.5 yet.

[SEJeff]

Feel free to give this a go via:

git pull [email protected]:SEJeff/kubernetes-client-python.git update-tox-config

And to run the flake8 task:

TOXENV=lint tox

I'm not sure of the best way to silence the errors in kubernetes.init, kubernetes.watch.init, and kubernetes.config.init. The most obvious thing is noqa: F401 on each line with the imports. Normally you could do from foo import bar and control the public bits with __all__, but that doesn't seemingly do the right thing with absolute imports + flake8.

So the choices I see are:

1. Disable F401 (unused import) on those lines and clutter up a few __init__.py files
2. Exclude the offending __init__.py files from flake8 entirely 😐
3. Disabling F401 globally 👎

I'm certainly not in love with 1, but it seems like the least evil. By design, there is no way to exclude specific errors globally from a specific file in flake8.

What do you prefer?

Once this is done, I'll enable it by default and I can open a PR for it.

[mbohlool]

init files suppose to be all imports (at least for us), I think (2) is better, just ignore init files.

[mbohlool]

Look like we have a 4th option:

4. Adding all in init files, for example for kubernetes/config/init.py:

__all__ = ["ConfigException", "load_incluster_config",
           "list_kube_config_contexts", "load_kube_config"]

I think this 4th option is sightly better than 2nd one.

[dims]

May be we should add the pep8/flake8 to the python-base

[iamneha]

/assign @iamneha

[k8s-ci-robot]

@iamneha: GitHub didn't allow me to assign the following users: iamneha.

Note that only kubernetes-client members and repo collaborators can be assigned.
For more information please see the contributor guide

In response to this:

/assign @iamneha

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[fejta-bot]

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten

[fejta-bot]

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

[k8s-ci-robot]

@fejta-bot: Closing this issue.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.