Skip to content

Fix for Python 3#4

Merged
psiinon merged 1 commit into
zaproxy:masterfrom
Woolworths:master
May 12, 2017
Merged

Fix for Python 3#4
psiinon merged 1 commit into
zaproxy:masterfrom
Woolworths:master

Conversation

@Woolworths

Copy link
Copy Markdown
Contributor

Otherwise it doesn't work..

@Woolworths

Copy link
Copy Markdown
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

Woolworths commented Jun 9, 2016

Copy link
Copy Markdown
Contributor Author

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

psiinon commented Jun 9, 2016

Copy link
Copy Markdown
Member

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

@Woolworths

Copy link
Copy Markdown
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

psiinon commented Jun 9, 2016

Copy link
Copy Markdown
Member

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

thc202 commented Jun 9, 2016

Copy link
Copy Markdown
Member

@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
Copy Markdown
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
Copy Markdown

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
Copy Markdown

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

thc202 commented Apr 14, 2017

Copy link
Copy Markdown
Member

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
Copy Markdown
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

thc202 commented Apr 17, 2017

Copy link
Copy Markdown
Member

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

thc202 commented May 4, 2017

Copy link
Copy Markdown
Member

Conflicts resolved, not ready to merge though.

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

thc202 commented May 5, 2017

Copy link
Copy Markdown
Member

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

@psiinon psiinon left a comment

Copy link
Copy Markdown
Member

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

psiinon commented May 12, 2017

Copy link
Copy Markdown
Member

@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
Copy Markdown
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

thc202 commented May 22, 2017

Copy link
Copy Markdown
Member

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