Skip to content

Python 3.x compatibility fixes#8

Merged
garethterrace merged 3 commits intomediaburst:masterfrom
bjornpost:python3
Jan 5, 2015
Merged

Python 3.x compatibility fixes#8
garethterrace merged 3 commits intomediaburst:masterfrom
bjornpost:python3

Conversation

@bjornpost
Copy link
Copy Markdown
Contributor

This fixes a couple of things to make sure the lib runs on Python 3 (tested on 3.4.2):

  • Implicit relative imports (import clockwork_http) have been made explicit (from . import clockwork_http)
  • Mixed use of indentation in one file (spaces and tabs), everything is converted to 4 spaces.
  • Urllib2 does no longer exist in Python 3 and is split in urllib.request and urllib.error. Fixed imports accordingly in a backward compatible way.

@bjornpost
Copy link
Copy Markdown
Contributor Author

Some refactoring ideas:

  • Load ApiTests.api_key from the current env (i.e. import os; os.environ['CLOCKWORK_API_KEY']), hardcoding it makes it easy to commit your api key by accident.
  • Throw an IllegalArgumentException when there is an unrecognized attribute in the attributes object in API.__build_sms_data()

@garethterrace
Copy link
Copy Markdown
Contributor

Hi Bjorn,

Really appreciate the work you've put in for us here - drop us a mail at hello@clockworksms.com and we can give you some free credit as a bit of a thanks.

We'll be pulling in these changes as soon as we can and the refactoring ideas we'll take a look at.

Thanks again

@wlcx
Copy link
Copy Markdown

wlcx commented Dec 27, 2014

Glad I checked the PRQs before I did this myself! 👍

@garethterrace
Copy link
Copy Markdown
Contributor

Happy with this - we'll update the package as soon as possible.

garethterrace pushed a commit that referenced this pull request Jan 5, 2015
Python 3.x compatibility fixes
@garethterrace garethterrace merged commit 415f692 into mediaburst:master Jan 5, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants