feat: add ADM support #1290
feat: add ADM support #1290
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1290 +/- ##
==========================================
- Coverage 99.95% 99.93% -0.02%
==========================================
Files 59 60 +1
Lines 9573 9847 +274
==========================================
+ Hits 9569 9841 +272
- Misses 4 6 +2
Continue to review full report at Codecov.
|
autopush/tests/test_integration.py
Outdated
| "crypto-key": [crypto_key], | ||
| "encryption": [salt], | ||
| "ttl": ["0"], | ||
| "" |
There was a problem hiding this comment.
kill this stray line (it just str concats into "content-encoding")
There was a problem hiding this comment.
Heh, had that in there for readability but then realized that i was the only person that cared.
autopush/router/adm.py
Outdated
| headers = { | ||
| "content-type": "application/x-www-form-urlencoded" | ||
| } | ||
| resp = self._request.post(url, data=body, headers=headers) |
There was a problem hiding this comment.
we definitely want to specify some kind of timeout to requests calls, GCM bridge uses timeout=10. Then you have to catch the Timeout exception somewhere (probably _route)
| data["data"] = payload | ||
| url = ("https://api.amazon.com/messaging/registrations" | ||
| "/{}/messages".format(reg_id)) | ||
| resp = self._request.post( |
There was a problem hiding this comment.
We can't use an async HTTP request? I know some of the router calls use third party tooling that we couldn't use the twisted http client with, but we should be able to here, right?
There was a problem hiding this comment.
I'd prefer to do the async requests as a separate issue, since that will span multiple files and fuzz exactly what this patch is supposed to address.
autopush/tests/test_integration.py
Outdated
| self._mock_reply.text = "Test error" | ||
| self._mock_reply.content = "Test content" | ||
|
|
||
| # This will call RouterException, which may halt the test thread. |
There was a problem hiding this comment.
I don't get this comment, why would RouterException halt the test thread?
There was a problem hiding this comment.
Since we'd be calling via the _agent, RouterException is passed up and not trapped, causing _agent to fail out and kill/fail the test. I was not able to trap for it. Thus calling the send directly with the raise trap. I can try to muck about more with _agent to try and handle that sort of thing, but seems a bit out of scope for this patch.
There was a problem hiding this comment.
That might be a case of twisted's TestCase failing if it sees a log.failure call made during the test. You can call self.flushLoggedErrors() at the end of tests to stop it from doing that
There was a problem hiding this comment.
@pjenvey Ah, cool. I'll try that.
Closes #1275
Closes #1275