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

[python-flask] Apply template tweaks to improve lint results #6826

Merged

Conversation

kenjones-cisco
Copy link
Contributor

PR checklist

  • Read the contribution guidelines.
  • Ran the shell script under ./bin/ to update Petstore sample so that CIs can verify the change. (For instance, only need to run ./bin/{LANG}-petstore.sh and ./bin/security/{LANG}-petstore.sh if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in .\bin\windows\.
  • Filed the PR against the correct branch: 3.0.0 branch for changes related to OpenAPI spec 3.0. Default: master.
  • Copied the technical committee to review the pull request if your PR is targeting a particular programming langauge.

Description of the PR

(details of the change, additional tests that have been done, reference to the issue for tracking, etc)

The linting results for the generated samples are as follows
where the first number is the BEFORE and the second is AFTER.

flaskConnexion          1843 vs. 20
flaskConnexion-python2  1841 vs. 19

For the complete details please see the following gist.
https://gist.github.com/kenjones-cisco/edc9d71ef7fd2bf23714ecbb693d52b3

@taxpon @frol @mbohlool @cbornet
CC @wing328

The linting results for the generated samples are as follows
where the first number is the BEFORE and the second is AFTER.

flaskConnexion          1843 vs. 20
flaskConnexion-python2  1841 vs. 19

For the complete details please see the following gist.
https://gist.github.com/kenjones-cisco/edc9d71ef7fd2bf23714ecbb693d52b3
@cbornet
Copy link
Contributor

cbornet commented Oct 27, 2017

Is it pylint output ?
Most of the templates come from the python client. Do you think you could apply the changes there also ?

@cbornet
Copy link
Contributor

cbornet commented Oct 27, 2017

Also, as you change the imports, can you check that the server starts well both from command line and from pyCharm ? This can be tricky sometimes.
I guess the CI will test that tox/nose works well.

@kenjones-cisco
Copy link
Contributor Author

I used flake8 for the linting.

As for pyCharm, I don't use that IDE so I'm not able to easily test it.

I can work on a separate PR for the python client as well.

@frol
Copy link
Contributor

frol commented Oct 27, 2017

LGTM

@kenjones-cisco
Copy link
Contributor Author

From command line:

{ flaskConnexion } task/handle-linting-errors » docker logs zen_allen
... OAuth2 token info URL missing. **IGNORING SECURITY REQUIREMENTS**
... OAuth2 token info URL missing. **IGNORING SECURITY REQUIREMENTS**
... OAuth2 token info URL missing. **IGNORING SECURITY REQUIREMENTS**
... OAuth2 token info URL missing. **IGNORING SECURITY REQUIREMENTS**
... OAuth2 token info URL missing. **IGNORING SECURITY REQUIREMENTS**
... OAuth2 token info URL missing. **IGNORING SECURITY REQUIREMENTS**
... OAuth2 token info URL missing. **IGNORING SECURITY REQUIREMENTS**
 * Running on http://0.0.0.0:8080/ (Press CTRL+C to quit)
{ flaskConnexion-python2 } task/handle-linting-errors » docker logs frosty_nobel
No handlers could be found for logger "connexion.operation"
 * Running on http://0.0.0.0:8080/ (Press CTRL+C to quit)

@wing328 wing328 merged commit 28d14e3 into swagger-api:master Oct 27, 2017
@wing328 wing328 modified the milestones: Future, v2.3.0 Oct 27, 2017
@kenjones-cisco kenjones-cisco deleted the task/handle-linting-errors branch October 27, 2017 14:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants