Summary

Add rate limiting for any API calls

Review Request #8839 — Created March 24, 2017 and discarded Aug. 1, 2017, 3:05 p.m.

Information

Owner

rkdhatt

Repository

Djblets

Branch

master

Bugs

Depends On

~~8698~~

Blocks

8868

Reviewers

Groups

djblets, students

People

Description

There's been a request to create a rate-limit for API calls. I have updated the ratelimit.py file that will allow rate limiting of API calls by either an authenticated user or an IP address.

Testing Done

None at the moment.

Issues

Description	From	Last Updated
local variable 'DEFAULT_RATE' is assigned to but never used	reviewbot	March 24, 2017, 7:43 p.m.
'django.test.utils.override_settings' imported but unused	reviewbot	March 24, 2017, 7:43 p.m.
'django.conf.settings' imported but unused	reviewbot	March 28, 2017, 2:25 p.m.
This should not be ALLCAPS. Only class-level and top-level constants are all-caps.	brennie	March 28, 2017, 2:21 p.m.
This can be simplified to: if not api_limit: #... else: # ...	brennie	March 28, 2017, 2:22 p.m.
Shouldnt the default param be self.DEFAULT_RATE ?	brennie	March 28, 2017, 2:22 p.m.
local variable 'DEFAULT_RATE' is assigned to but never used	reviewbot	March 24, 2017, 7:43 p.m.
This should be the default_rate	brennie	March 28, 2017, 2:23 p.m.
'django.conf.settings' imported but unused	reviewbot	March 24, 2017, 7:43 p.m.
Re-flow this to 79ch wide.	brennie	March 28, 2017, 2:24 p.m.
This should use a constant instead of repeating 5/m everywhere.	brennie	March 28, 2017, 2:25 p.m.
undefined name 'WebAPIResourceRateLimitMixin'	reviewbot	March 31, 2017, 10:56 a.m.
undefined name 'increment'	reviewbot	March 31, 2017, 10:56 a.m.
undefined name '_'	reviewbot	March 31, 2017, 10:58 a.m.
'djblets.webapi.resources.mixins.api_ratelimit.WebAPIResourceMixin' imported but unused	reviewbot	March 31, 2017, 11:58 a.m.
undefined name 'increment'	reviewbot	March 31, 2017, 10:58 a.m.
undefined name '_'	reviewbot	March 31, 2017, 10:58 a.m.
'djblets.auth.ratelimit.Rate' imported but unused	reviewbot	April 1, 2017, 10:23 a.m.
'djblets.auth.ratelimit.is_ratelimited' imported but unused	reviewbot	April 1, 2017, 10:26 a.m.
'warnings' imported but unused	reviewbot	April 1, 2017, 10:23 a.m.
'django.db.models.Model' imported but unused	reviewbot	April 1, 2017, 10:23 a.m.
'django.utils.six' imported but unused	reviewbot	April 1, 2017, 10:24 a.m.
'django.utils.translation.ugettext_lazy as _' imported but unused	reviewbot	April 1, 2017, 10:24 a.m.
'djblets.auth.ratelimit.Rate' imported but unused	reviewbot	April 1, 2017, 10:24 a.m.
'djblets.webapi.resources.registry.unregister_resource_for_model' imported but unused	reviewbot	April 1, 2017, 10:25 a.m.
'djblets.webapi.resources.registry.register_resource_for_model' imported but unused	reviewbot	April 1, 2017, 10:25 a.m.
local variable 'result' is assigned to but never used	reviewbot	April 1, 2017, 10:25 a.m.
'djblets.auth.ratelimit.Rate' imported but unused	reviewbot	April 1, 2017, 10:33 a.m.
'djblets.webapi.resources.registry.unregister_resource' imported but unused	reviewbot	April 3, 2017, 11:21 a.m.

JSHint passed.

PEP8 Style Checker internal error.

Pyflakes failed.

Pyflakes

djblets/auth/ratelimit.py (Diff revision 1)
The issue has been dropped. Show all issues
```
 local variable 'DEFAULT_RATE' is assigned to but never used
```
djblets/auth/tests/test_ratelimit.py (Diff revision 1)
The issue has been dropped. Show all issues
```
 'django.test.utils.override_settings' imported but unused
```
djblets/webapi/resources/mixins/api_ratelimit.py (Diff revision 1)
The issue has been resolved. Show all issues
```
 'django.conf.settings' imported but unused
```

Summary:

-	Added new ratelimit file for rate limiting authentication (in progress)
+	Add rate limiting for any API calls (in progress)

Summary:

-	Add rate limiting for any API calls (in progress)
+	[WIP] Add rate limiting for any API calls

Change Summary:

Accidentally uploaded both patches to same review request - fixed.

Diff:

Revision 2 (+88 -14)

Show changes

	djblets/settings.py
	djblets/auth/ratelimit.py
	djblets/webapi/resources/mixins/api_ratelimit.py
	tests/settings.py

Checks run (1 failed, 1 succeeded, 1 failed with error)

JSHint passed.

PEP8 Style Checker internal error.

Pyflakes failed.

Pyflakes

djblets/auth/ratelimit.py (Diff revision 2)
The issue has been dropped. Show all issues
```
 local variable 'DEFAULT_RATE' is assigned to but never used
```
djblets/webapi/resources/mixins/api_ratelimit.py (Diff revision 2)
The issue has been dropped. Show all issues
```
 'django.conf.settings' imported but unused
```

djblets/auth/ratelimit.py (Diff revision 2)

The issue has been resolved. Show all issues

This should not be ALLCAPS. Only class-level and top-level constants are all-caps.

djblets/auth/ratelimit.py (Diff revision 2)

The issue has been resolved. Show all issues

This can be simplified to:

if not api_limit:
    #...
else:
    # ...

djblets/auth/ratelimit.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Shouldnt the default param be self.DEFAULT_RATE ?
```
djblets/auth/ratelimit.py (Diff revision 2)
The issue has been resolved. Show all issues
```
This should be the default_rate
```
djblets/webapi/resources/mixins/api_ratelimit.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Re-flow this to 79ch wide.
```
djblets/webapi/resources/mixins/api_ratelimit.py (Diff revision 2)
The issue has been resolved. Show all issues
```
This should use a constant instead of repeating 5/m everywhere.
```

Commit:

-	18936d77b24c18efdb1f351a58436d2edf8c0458
+	dc795ba46ebe4549d55a7c057963e299597c0e35

Diff:

Revision 3 (+101 -14)

Show changes

	djblets/settings.py
	djblets/auth/ratelimit.py
	djblets/webapi/resources/base.py
	djblets/webapi/resources/mixins/api_ratelimit.py
	tests/settings.py

Checks run (1 failed, 1 succeeded, 1 failed with error)

JSHint passed.

PEP8 Style Checker internal error.

Pyflakes failed.

Pyflakes

djblets/webapi/resources/base.py (Diff revision 3)
The issue has been dropped. Show all issues
```
 undefined name 'WebAPIResourceRateLimitMixin'
```
djblets/webapi/resources/mixins/api_ratelimit.py (Diff revision 3)
The issue has been resolved. Show all issues
```
 undefined name 'increment'
```
djblets/webapi/resources/mixins/api_ratelimit.py (Diff revision 3)
The issue has been resolved. Show all issues
```
 undefined name '_'
```

Diff:

Revision 4 (+101 -13)

Show changes

	djblets/settings.py
	djblets/auth/ratelimit.py
	djblets/webapi/resources/base.py
	djblets/webapi/resources/mixins/api_ratelimit.py
	tests/settings.py

Checks run (1 failed, 1 succeeded, 1 failed with error)

JSHint passed.

PEP8 Style Checker internal error.

Pyflakes failed.

Pyflakes

djblets/webapi/resources/base.py (Diff revision 4)

The issue has been resolved. Show all issues

 'djblets.webapi.resources.mixins.api_ratelimit.WebAPIResourceMixin' imported but unused

djblets/webapi/resources/mixins/api_ratelimit.py (Diff revision 4)
The issue has been resolved. Show all issues
```
 undefined name 'increment'
```
djblets/webapi/resources/mixins/api_ratelimit.py (Diff revision 4)
The issue has been resolved. Show all issues
```
 undefined name '_'
```

Change Summary:

Added test for new mixin, along with documentation.

Diff:

Revision 5 (+248 -13)

Show changes

	djblets/settings.py
	djblets/auth/ratelimit.py
	djblets/webapi/resources/mixins/api_ratelimit.py
	djblets/webapi/tests/test_api_ratelimit.py
	tests/settings.py

Checks run (1 failed, 1 succeeded, 1 failed with error)

JSHint passed.

PEP8 Style Checker internal error.

Pyflakes failed.

Pyflakes

djblets/webapi/resources/mixins/api_ratelimit.py (Diff revision 5)
The issue has been resolved. Show all issues
```
 'djblets.auth.ratelimit.Rate' imported but unused
```
djblets/webapi/resources/mixins/api_ratelimit.py (Diff revision 5)
The issue has been resolved. Show all issues
```
 'djblets.auth.ratelimit.is_ratelimited' imported but unused
```
djblets/webapi/tests/test_api_ratelimit.py (Diff revision 5)
The issue has been resolved. Show all issues
```
 'warnings' imported but unused
```
djblets/webapi/tests/test_api_ratelimit.py (Diff revision 5)
The issue has been resolved. Show all issues
```
 'django.db.models.Model' imported but unused
```
djblets/webapi/tests/test_api_ratelimit.py (Diff revision 5)
The issue has been resolved. Show all issues
```
 'django.utils.six' imported but unused
```
djblets/webapi/tests/test_api_ratelimit.py (Diff revision 5)
The issue has been resolved. Show all issues
```
 'django.utils.translation.ugettext_lazy as _' imported but unused
```
djblets/webapi/tests/test_api_ratelimit.py (Diff revision 5)
The issue has been resolved. Show all issues
```
 'djblets.auth.ratelimit.Rate' imported but unused
```
djblets/webapi/tests/test_api_ratelimit.py (Diff revision 5)
The issue has been resolved. Show all issues
```
 'djblets.webapi.resources.registry.unregister_resource_for_model' imported but unused
```
djblets/webapi/tests/test_api_ratelimit.py (Diff revision 5)
The issue has been resolved. Show all issues
```
 'djblets.webapi.resources.registry.register_resource_for_model' imported but unused
```
djblets/webapi/tests/test_api_ratelimit.py (Diff revision 5)
The issue has been resolved. Show all issues
```
 local variable 'result' is assigned to but never used
```

Diff:

Revision 6 (+240 -13)

Show changes

	djblets/settings.py
	djblets/auth/ratelimit.py
	djblets/webapi/resources/mixins/api_ratelimit.py
	djblets/webapi/tests/test_api_ratelimit.py
	tests/settings.py

Checks run (1 failed, 1 succeeded, 1 failed with error)

JSHint passed.

PEP8 Style Checker internal error.

Pyflakes failed.

Pyflakes

djblets/webapi/resources/mixins/api_ratelimit.py (Diff revision 6)
The issue has been resolved. Show all issues
```
 'djblets.auth.ratelimit.Rate' imported but unused
```

Diff:

Revision 7 (+240 -13)

Show changes

	djblets/settings.py
	djblets/auth/ratelimit.py
	djblets/webapi/resources/mixins/api_ratelimit.py
	djblets/webapi/tests/test_api_ratelimit.py
	tests/settings.py

Checks run (2 succeeded, 1 failed with error)

JSHint passed.

PEP8 Style Checker internal error.

Pyflakes passed.

Diff:

Revision 8 (+241 -13)

Show changes

	djblets/settings.py
	djblets/auth/ratelimit.py
	djblets/webapi/resources/mixins/api_ratelimit.py
	djblets/webapi/tests/test_api_ratelimit.py
	tests/settings.py

Checks run (2 succeeded, 1 failed with error)

JSHint passed.

PEP8 Style Checker internal error.

Pyflakes passed.

Diff:

Revision 9 (+241 -13)

Show changes

	djblets/settings.py
	djblets/auth/ratelimit.py
	djblets/webapi/resources/mixins/api_ratelimit.py
	djblets/webapi/tests/test_api_ratelimit.py
	tests/settings.py

Checks run (2 succeeded, 1 failed with error)

JSHint passed.

PEP8 Style Checker internal error.

Pyflakes passed.

Diff:

Revision 10 (+235 -13)

Show changes

	djblets/settings.py
	djblets/auth/ratelimit.py
	djblets/webapi/resources/mixins/api_ratelimit.py
	djblets/webapi/tests/test_api_ratelimit.py
	tests/settings.py

Checks run (1 failed, 1 succeeded, 1 failed with error)

JSHint passed.

PEP8 Style Checker internal error.

Pyflakes failed.

Pyflakes

djblets/webapi/tests/test_api_ratelimit.py (Diff revision 10)
The issue has been dropped. Show all issues
```
 'djblets.webapi.resources.registry.unregister_resource' imported but unused
```

Diff:

Revision 11 (+234 -13)

Show changes

	djblets/settings.py
	djblets/auth/ratelimit.py
	djblets/webapi/resources/mixins/api_ratelimit.py
	djblets/webapi/tests/test_api_ratelimit.py
	tests/settings.py

Checks run (2 succeeded, 1 failed with error)

JSHint passed.

PEP8 Style Checker internal error.

Pyflakes passed.

Summary:

-	[WIP] Add rate limiting for any API calls
+	Add rate limiting for any API calls

Status: Discarded

Change Summary:

Discarding in favor of https://reviews.reviewboard.org/r/9114/