-
-
Notifications
You must be signed in to change notification settings - Fork 87
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 for Python 3 #4
Conversation
Check out my branch -- Python3 is fully working along with Python2. Because I changed the folder structure, you would want to update the |
My version is now FULLY WORKING AND TESTED for Python2 and Python3. However, to commit this, you need to change the setup.py accordingly, and change the Python3APIImpl to:
For the |
Are you going to submit a new PR for this? |
I did submit a PR, but decided that it would be best left to the ZAP guys to finish it off. I submitted my PR so you guys could get ideas for how I managed to have compatibility with Python2 + Python3 (again, this works for both). This works as a library for me, however there are things that need to be changed (such as the |
Please do - I wrote the original python API, but I'm not a python programmer. But I can definitely test and comment on a PR, and we can then merge it when its ready :) |
@Woolworths did you take a look at #1 (and #2)? Could check/review/comment about the proposed change(s)? |
@thc202 Just checked it out and I will write my review of it on his PR. |
Hi @psiinon ! Would you test and merge this PR in the future? I would like to use this API in Python 3 environment. It will be fantastic if this official API package works without patch. :D |
Has anyone taken a shot at reconciling the conflicts and "three-proofing" this? It seems the codebase has been updated to support new API functionality and the patch is out of date now. |
I don't think so, but we can't resolve the conflicts in this branch though. @Woolworths could you give permission [1] so that we can fix the conflicts? [1] https://help.github.com/articles/allowing-changes-to-a-pull-request-branch-created-from-a-fork/ |
@thc202 Sure! I will come back to this shortly and ensure that it is working with the master branch. Correct me if I'm wrong but this project is automatically generated from the Python3APIImpl class? If so changes would need to be made to that class as mentioned in my second comment. It has been a while since I have looked through the source code, my apologies. |
Great, thank you! OK, I'll take a look at the generator to ensure it produces the new/expected code (when ready I'll open a pull request linking to this one). |
Conflicts resolved, not ready to merge though. |
f73c043
to
c451244
Compare
Change PythonAPIGenerator to use the six library, for compatibility with Python 2 and 3. Also, tweak it to not add unnecessary empty lines at the end of the generated files. For/from zaproxy/zap-api-python#4 - Fix for Python 3
Change PythonAPIGenerator to use the six library, for compatibility with Python 2 and 3. Also, tweak it to be more compliant with PEP8. For/from zaproxy/zap-api-python#4 - Fix for Python 3
Change library to support Python 3: - Update print statements; - Use six library in the generated code. Add Python 3 (3.3) to TravisCI build configuration file. Add API of OpenAPI add-on. Bump version to 0.0.10.
Ready for review/merge. Tested with Python 2.7 and 3.4, installed and run fine in both cases. Feedback appreciated... (Reverted the package and class rename, to allow to keep using the same code.) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested with python2 and python3, all looking good
@Woolworths Many thanks for doing this - how would you like to be credited? |
@psiinon No problem! I guess the most suitable would be my name: "Jean Abed"! |
Done in zaproxy/zap-core-help#103. Thank you! |
Change PythonAPIGenerator to use the six library, for compatibility with Python 2 and 3. Also, tweak it to be more compliant with PEP8. For/from zaproxy/zap-api-python#4 - Fix for Python 3
Otherwise it doesn't work..