Conversation
|
Duh! And I noticed it now that the endpoint is deprecated 🏅 |
|
I would modify this endpoint (you can keep the work done to date) to do a deprecation warning (docs). |
|
@richleland added the deprecation notice |
sparkpost/transmissions.py
Outdated
| """ | ||
| results = self.request('GET', self.uri) | ||
| return results | ||
| warnings.warn('deprecated', DeprecationWarning) |
There was a problem hiding this comment.
I would customize the message rather than put in deprecated. For instance we could say it will be removed from the API soon. After looking a little more, you might want to use PendingDeprecationWarning. Curious - what does this look like at the command line for users?
There was a problem hiding this comment.
I put a giant message initially but seemed too much. something like
This endpoint is deprecated. For details, check https://developers.sparkpost.com/api/transmissions.html#transmissions-list-get
To me it seemed verbose. But if you find this (or modify as you want) helpful, I can put that.
Also the endpoint is already deprecated; so PendingDeprecationWarning may not be correct.
There was a problem hiding this comment.
Good point on it already being deprecated, let's keep it as DeprecationWarning. Maybe use the URL shortener for that message. Otherwise the message you suggested seems OK to me.
There was a problem hiding this comment.
i can shorten; i hope it won't feel creepy to users :).
test/test_transmissions.py
Outdated
| import os | ||
| import tempfile | ||
| import warnings | ||
| from mock import patch |
There was a problem hiding this comment.
Move this below the external libraries. The ordering should follow this format:
- Standard library (built-in)
imports - Standard library (built-in)
froms
line break
- External library
imports - External library
froms
line break
- Local
imports - Local
froms
In each the imports/froms should be in alphabetical order.
sparkpost/transmissions.py
Outdated
| """ | ||
| warnings.warn('deprecated', DeprecationWarning) | ||
| warn_msg = 'This endpoint is deprecated. For details, ' | ||
| 'check http://bit.ly/2lAKj60.' |
There was a problem hiding this comment.
we have our own URL shortener: https://sparkpo.st/mklink.php
sparkpost/transmissions.py
Outdated
| import copy | ||
| import json | ||
| import warnings | ||
|
|
There was a problem hiding this comment.
no line break needed here - email is in the standard lib.
rajumsys
force-pushed
the
issue133
branch
2 times, most recently
from
eddbe00 to
7153d37
Compare
sparkpost/base.py
Outdated
| 'Content-Type': 'application/json', | ||
| 'Authorization': self.api_key | ||
| } | ||
| print uri, kwargs |
2 participants
Handles #133