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

Fix for Python 3 #4

Merged
merged 1 commit into from
May 12, 2017
Merged

Fix for Python 3 #4

merged 1 commit into from
May 12, 2017

Conversation

Woolworths
Copy link
Contributor

Otherwise it doesn't work..

@Woolworths
Copy link
Contributor Author

Check out my branch -- Python3 is fully working along with Python2. Because I changed the folder structure, you would want to update the setup.py if you were to push it.

@Woolworths
Copy link
Contributor Author

Woolworths commented Jun 9, 2016

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:

  • import six
  • change next() -> six.next()
  • change .iteritems -> six.iteritems()

For the __init__.py I changed urllib to requests as it works better for Python2 + Python3 and it looks cleaner.
To run, read the README.md (pull, run and inspect) then get the IpAddress variable given back and use that as a proxy. Enjoy!

@Woolworths Woolworths closed this Jun 9, 2016
@psiinon
Copy link
Member

psiinon commented Jun 9, 2016

Are you going to submit a new PR for this?
Cheers

@Woolworths
Copy link
Contributor Author

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 setup.py variables and the Python3APIImpl). If you would like though, I could submit it.

@psiinon
Copy link
Member

psiinon commented Jun 9, 2016

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 :)

@thc202
Copy link
Member

thc202 commented Jun 9, 2016

@Woolworths did you take a look at #1 (and #2)? Could check/review/comment about the proposed change(s)?
(Both pull requests seem to provide support for Python 3.)

@Woolworths
Copy link
Contributor Author

@thc202 Just checked it out and I will write my review of it on his PR.

@Woolworths Woolworths reopened this Jun 10, 2016
@wdhongtw
Copy link

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

@ravenium
Copy link

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.

@thc202
Copy link
Member

thc202 commented Apr 14, 2017

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/

@Woolworths
Copy link
Contributor Author

@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.

@thc202
Copy link
Member

thc202 commented Apr 17, 2017

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).

@thc202
Copy link
Member

thc202 commented May 4, 2017

Conflicts resolved, not ready to merge though.

@thc202 thc202 force-pushed the master branch 6 times, most recently from f73c043 to c451244 Compare May 4, 2017 14:23
thc202 added a commit to thc202/zaproxy that referenced this pull request May 4, 2017
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
thc202 added a commit to thc202/zaproxy that referenced this pull request May 5, 2017
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.
@thc202
Copy link
Member

thc202 commented May 5, 2017

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.)

Copy link
Member

@psiinon psiinon left a 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

@psiinon psiinon merged commit d5e83fe into zaproxy:master May 12, 2017
@psiinon
Copy link
Member

psiinon commented May 12, 2017

@Woolworths Many thanks for doing this - how would you like to be credited?
https://github.com/zaproxy/zap-core-help/wiki/HelpCredits

@Woolworths
Copy link
Contributor Author

@psiinon No problem! I guess the most suitable would be my name: "Jean Abed"!

thc202 added a commit to thc202/zap-core-help that referenced this pull request May 18, 2017
@thc202
Copy link
Member

thc202 commented May 22, 2017

Done in zaproxy/zap-core-help#103. Thank you!

juhakivekas pushed a commit to juhakivekas/zaproxy that referenced this pull request Sep 29, 2017
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants