fastly / fastly-py

A Fastly API client for Python
https://pypi.org/project/fastly/
MIT License
77 stars 59 forks source link

Migrated Fastly client to Python 3 #31

Closed aryzhov closed 6 years ago

aryzhov commented 6 years ago

My company needs to use billing and stats Fastly APIs. I discovered this client but found that it's not compatible with Python 3 and that it does not have support for the APIs that we need. I ran the 2to3 utility to convert the code and ran unit tests to ensure that it works. I am planning on adding more functionality to the client which I will submit as a separate pull request. This pull request is for Python 3 migration only.

Please note that I changed the version to 3.0.0. I am not sure what the version should be, please advise.

Please note also that this version breaks Python 2 compatibility. Python 2 is the past and many libraries are abandoning it, and I certainly don't have the bandwidth to support it. Given between the two alternatives: Python 2 only and Python 3 only, Python 3 is certainly better, therefore I think it's reasonable to migrate the client to Python 3 at this point.

amyrlam commented 6 years ago

@aryzhov Hey, sorry this has been open for so long! It looks like we will need to still support Python 2 from the download stats. Hope to have more info from the Fastly side soon re: this repo.

Relates to: https://github.com/fastly/fastly-py/issues/17

verkaufer commented 6 years ago

If this PR can be updated to include backwards compatibility with Python 2 would it fast-track a merge?

psbanka commented 6 years ago

Sorry for the delay in reviewing. I'll take a look at this today and tomorrow!

psbanka commented 6 years ago

@verkaufer – do you have time to make this backwards compatible with Python2? If so, yes – we could fast-track the merge.

aryzhov commented 6 years ago

I just ran the unit tests against 2.7.10 and they went through ok. This is just a matter of choosing the minimum version of Python 2 that you need to support. Another thing: since this request didn't get attention in time, I had to make my own Fastly client just for Billing and Stats: https://github.com/aryzhov/fastly-client Let me know which version of Python 2 to put in setup.py and I'll update the pull request with that line.

verkaufer commented 6 years ago

Given that Python 2 will stop receiving support in about 18 months as of writing this, we should go with the most recent 2.7 (which looks to be 2.7.13)

@psbanka I can work on getting python 2.7 compatibility. Can't put a definite delivery on this, but probably next week.

aryzhov commented 6 years ago

I just realized that I have deleted the repo which I submitted my pull request from. You may accept my request as-is (I tested it with Python 2 and it works) and add Python 2.7 to setup.py prior to releasing the next version, or feel free to reject it, as this feature is no longer critical for me.

verkaufer commented 6 years ago

I am working on a separate fork that I'll make a PR for. Until I get the PR created let's keep this open for reference sake.

verkaufer commented 6 years ago

This can probably be closed now that my PR is open. Let's move discussion over there.