rate_limit: Adjust keys() of some RateLimitedObjects.

type().__name__ is sufficient, and much readable than type(), so it's
better to use the former for keys.
We also make the classes consistent in forming the keys in the format
type(self).__name__:identifier and adjust logger.warning and statsd to
take advantage of that and simply log the key().
This commit is contained in:
Mateusz Mandera
2020-03-06 10:56:36 +01:00
committed by Tim Abbott
parent 2c6b1fd575
commit 4e9f77a6c4
6 changed files with 9 additions and 9 deletions

View File

@@ -309,7 +309,7 @@ class RateLimitedPasswordResetByEmail(RateLimitedObject):
return "Email: {}".format(self.email)
def key(self) -> str:
return "{}:{}".format(type(self), self.email)
return "{}:{}".format(type(self).__name__, self.email)
def rules(self) -> List[Tuple[int, int]]:
return settings.RATE_LIMITING_RULES['password_reset_form_by_email']

View File

@@ -445,7 +445,7 @@ class RateLimitedRealmMirror(RateLimitedObject):
super().__init__()
def key(self) -> str:
return "emailmirror:{}:{}".format(type(self.realm), self.realm.id)
return "{}:{}".format(type(self).__name__, self.realm.string_id)
def rules(self) -> List[Tuple[int, int]]:
return settings.RATE_LIMITING_MIRROR_REALM_RULES

View File

@@ -113,7 +113,7 @@ class RateLimitedUser(RateLimitedObject):
return "Id: {}".format(self.user.id)
def key(self) -> str:
return "{}:{}:{}".format(type(self.user), self.user.id, self.domain)
return "{}:{}:{}".format(type(self).__name__, self.user.id, self.domain)
def rules(self) -> List[Tuple[int, int]]:
# user.rate_limits are general limits, applicable to the domain 'api_by_user'
@@ -342,14 +342,13 @@ class RedisRateLimiterBackend(RateLimiterBackend):
ratelimited, time = cls.is_ratelimited(entity)
if ratelimited:
statsd.incr("ratelimiter.limited.%s.%s" % (type(entity), str(entity)))
statsd.incr("ratelimiter.limited.%s" % (entity.key(),))
else:
try:
cls.incr_ratelimit(entity)
except RateLimiterLockingException:
logger.warning("Deadlock trying to incr_ratelimit for %s:%s" % (
type(entity).__name__, str(entity)))
logger.warning("Deadlock trying to incr_ratelimit for %s" % (entity.key(),))
# rate-limit users who are hitting the API so hard we can't update our stats.
ratelimited = True

View File

@@ -121,5 +121,5 @@ class RateLimitTests(ZulipTestCase):
side_effect=RateLimiterLockingException):
result = self.send_api_message(user, "some stuff")
self.assertEqual(result.status_code, 429)
mock_warn.assert_called_with("Deadlock trying to incr_ratelimit for RateLimitedUser:Id: %s"
mock_warn.assert_called_with("Deadlock trying to incr_ratelimit for RateLimitedUser:%s:api_by_user"
% (user.id,))

View File

@@ -411,7 +411,8 @@ class WorkerTest(ZulipTestCase):
fake_client.queue.append(('email_mirror', data[0]))
worker.start()
self.assertEqual(mock_mirror_email.call_count, 4)
expected_warn = "Deadlock trying to incr_ratelimit for RateLimitedRealmMirror:zulip"
expected_warn = "Deadlock trying to incr_ratelimit for RateLimitedRealmMirror:%s" % (
realm.string_id,)
mock_warn.assert_called_with(expected_warn)
def test_email_sending_worker_retries(self) -> None:

View File

@@ -186,7 +186,7 @@ class RateLimitedAuthenticationByUsername(RateLimitedObject):
return "Username: {}".format(self.username)
def key(self) -> str:
return "{}:{}".format(type(self), self.username)
return "{}:{}".format(type(self).__name__, self.username)
def rules(self) -> List[Tuple[int, int]]:
return rate_limiting_rules