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

Python3 supported version #4

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

MiralParmar21
Copy link

No description provided.

@vighneshtrivedi2004
Copy link
Contributor

Hello Miral,

I am from Paubox team and I have below feedback points after reviewing your PR:

  1. PR changes are using byte prefix => b'' in test cases. This may not be required. Please kindly suggest reasons for making this change.
  2. PR changes have introduced new dependency "config" in requirements.txt file. This may not be required. Please remove dependency if it is not required.
  3. In general, code changes done in PR will not break on Python v2.7, v3.0 and v3.6. But it also does not fix existing issues already present on Python SDK with respect to Python v3.0 and v3.6.

Have you run test cases successfully for entire SDK after integrating PR changes on v2.7, v3.0 and v3.6?

Please let us know your feedback on above questions.

Regards,
Vighnesh

@MiralParmar21
Copy link
Author

Hi Vighnesh,

Thanks for the feedback.

  1. In python 3, bytes and string data-types are different. However in python 2, both are the same. The function b64encoder() expected byte type argument. So, I added b'' in test cases. The change is backward-compatible and will not break anything in Python 2.
  2. I removed "config" from the requirements.txt as per your suggestion.
  3. Yes, I have run test cases in both python versions successfully.

Thanks,
Miral

@vighneshtrivedi2004
Copy link
Contributor

Hi Miral,

I have run the SDK with your changes on Python v3.0 and v3.6 and I have not been able to successfully run the SDK unit tests. Would you please share your test results? Also, have you tried a scenario with attachment file? SDK breaks on v3.6 when attachment is added to the API request.

Below is the error log:
E:\Projects\PythonProject\PythonSDK\Modified Code with Python3 changes\paubox-python-python3-supported-version>python -V
Python 3.6.0
E:\Projects\PythonProject\PythonSDK\Modified Code with Python3 changes\paubox-python-python3-supported-version>python .\test\test_paubox.py
test_mail_class_all_headers (main.TestPaubox)
Test Mail class formatting ... ok
test_mail_class_setters (main.TestPaubox)
Test Mail class setters functionality ... ok
test_retrieve_disposition (main.TestPaubox)
Test get email disposition functionality ... ERROR
test_sending (main.TestPaubox)
Test send email functionality ... ERROR

ERROR: test_retrieve_disposition (main.TestPaubox)
Test get email disposition functionality

Traceback (most recent call last):
File ".\test\test_paubox.py", line 166, in test_retrieve_disposition
source_tracking_id = send_response.to_dict['sourceTrackingId']
File "E:\Projects\PythonProject\PythonSDK\Modified Code with Python3 changes\paubox-python-python3-supported-version\paubox\paubox.py", line 51, in to_dict
return json.loads(self.text.decode('utf-8'))
AttributeError: 'str' object has no attribute 'decode'

ERROR: test_sending (main.TestPaubox)
Test send email functionality

Traceback (most recent call last):
File ".\test\test_paubox.py", line 146, in test_sending
paubox_response = paubox_client.send(mail.get())
File "E:\Projects\PythonProject\PythonSDK\Modified Code with Python3 changes\paubox-python-python3-supported-version\paubox\paubox.py", line 82, in send
File "E:\Projects\PythonProject\PythonSDK\Modified Code with Python3 changes\paubox-python-python3-supported-version\paubox\paubox.py", line 82, in send
response = requests.post(url, json=mail, headers=headers)
File "C:\Python36\lib\site-packages\requests\api.py", line 119, in post
return request('post', url, data=data, json=json, **kwargs)
File "C:\Python36\lib\site-packages\requests\api.py", line 61, in request
return session.request(method=method, url=url, **kwargs)
File "C:\Python36\lib\site-packages\requests\sessions.py", line 516, in request
prep = self.prepare_request(req)
File "C:\Python36\lib\site-packages\requests\sessions.py", line 459, in prepare_request
hooks=merge_hooks(request.hooks, self.hooks),
File "C:\Python36\lib\site-packages\requests\models.py", line 317, in prepare
self.prepare_body(data, files, json)
File "C:\Python36\lib\site-packages\requests\models.py", line 467, in prepare_body
body = complexjson.dumps(json)
File "C:\Python36\lib\json_init_.py", line 231, in dumps
return _default_encoder.encode(obj)
File "C:\Python36\lib\json\encoder.py", line 199, in encode
chunks = self.iterencode(o, _one_shot=True)
File "C:\Python36\lib\json\encoder.py", line 257, in iterencode
return _iterencode(o, 0)
File "C:\Python36\lib\json\encoder.py", line 180, in default
o.class.name)
TypeError: Object of type 'bytes' is not JSON serializable.

Regards,
Vighnesh

@vighneshtrivedi2004
Copy link
Contributor

I had also tried to setup python 3 with pip on my machine, but unable to install pip. How have you done setup for Python SDK for v3.0?

@brunswick79
Copy link

For reference, I have forked your SDK and have it working in a python3 project. I used most of the changes in this PR except the requests library as I already had it as part of the project. The error you are seeing above Vighnesh is from helpers/mail.py lines 51-56. There is no need to encode the html in python3. I've removed those lines for my project, but if you want to keep python2/3 compatible you should probably put in a conditional. Hope that helps.

@dieselmachine
Copy link

The issue in helpers.mail is that python3 doesn't have an equivalent to basestring, and six.string_types is (basestring,) in py2, and (str,) in py3. Those are not equivalent. Py2 basestring will match bytes and text, Py3 str will not.

Despite the difference in meaning, the 2to3 conversion code will blindly convert basestring to str and break things.

And the maintainers have no interest whatsoever in fixing that

The closest equivalent in py3 would be (bytes, str) (cherrypy uses that, among others). It would be better to just set basestring to (str, bytes) if in Py3, because six won't let you do what you want here.

@niwong
Copy link
Contributor

niwong commented Jun 4, 2021

Hey folks,

The @Paubox engineering team released a Python 3-specific SDK for the Paubox Email API, and that repository can be found here:
https://github.com/Paubox/paubox-python3.

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.

6 participants