Conversation
bentsku
added
aws:sns
|
FYI, we might have some overlap with #11855 |
LocalStack Community integration with Pro 2 files ± 0 2 suites ±0 1h 5m 19s ⏱️ - 38m 42s Results for commit 5c26b71. ± Comparison against base commit 5b44747. This pull request removes 1397 tests. |
thrau
left a comment
thrau
left a comment
There was a problem hiding this comment.
LGTM! one minor nit
i think it would be great to solve this more generically across endpoints moving forward, but good for now to get some initial readings!
| def _wrapper(self, *args, **kwargs): | ||
| self.count_usage() | ||
| return f(self, *args, **kwargs) |
There was a problem hiding this comment.
perhaps we don't need self?
| def _wrapper(self, *args, **kwargs): | |
| self.count_usage() | |
| return f(self, *args, **kwargs) | |
| def _wrapper(*args, **kwargs): | |
| self.count_usage() | |
| return f(*args, **kwargs) |
There was a problem hiding this comment.
I think this will complain, we do need self passed, it failed before I added it 😄
edit: oops, misread the suggested change. let me check 👀
There was a problem hiding this comment.
If I don't pass self, then I need to extract it from args right, otherwise self.count_usage() will raise an exception as it wouldn't be in scope?
There was a problem hiding this comment.
oh ,whoops, didn't see that there, of course your right!
There was a problem hiding this comment.
I'm not too fond of this either tbh, for a one line change I could just call self.count_usage() inside the method itself... it's the same as decorating it 😬 it just feels wrong to add unrelated code inside the method logic
I could also maybe auto-wraps methods starting with on_ in a similar way to the resource thing. Anyway... this will do until we rework it!
3 participants
Motivation
As part of our effort to improve our analytics, this PR implements usage counters for SNS internal routes.
We can now properly have usage data on SNS internal routes invocations, to understand their usage better.
Usage is separated on what the internal route is targeting, be it Platform Endpoint messages, SMS messages or Subscription Tokens (useful for example for email subscriptions).
Changes