feat: Add support for flexible throttling intervals #9079

PravinKamble123 · 2023-08-20T17:54:29Z

This commit introduces the ability to set custom time intervals for throttling, allowing users to specify intervals like per 5 minutes, per 2 hours, and per 5 days. This enhancement provides more flexibility in controlling request rates.

Note: Before submitting this pull request, please review our contributing guidelines.

Description

Please describe your pull request. If it fixes a bug or resolves a feature request, be sure to link to that issue. When linking to an issue, please use refs #... in the description of the pull request.

This commit introduces the ability to set custom time intervals for throttling, allowing users to specify intervals like per 5 minutes, per 2 hours, and per 5 days. This enhancement provides more flexibility in controlling request rates.

sevdog

This could be an interesting feature. However the code must be cleaned and also docs and example must be written for this.

sevdog · 2023-08-21T12:32:07Z

rest_framework/utils/throttling_duration_parser.py

Is there any reason for having this ouside the throttling package?

I thought creating file inside the utils folder will more organised.

The point is that this method does not have any other use case, so IMO having an extra file for this does not seem good choice.

It could make sense if there was the need to have an utility method which needs to be used by throttling package and by others, as of new there is no need to have this in an extra package.

make sense👍

sevdog · 2023-08-21T12:33:04Z

rest_framework/utils/throttling_duration_parser.py

+    quantity_unit_dict = {}
+    while i < len(quantity_unit_string) and quantity_unit_string[i].isnumeric():
+        i += 1
+    if i == 0:
+        quantity_unit_dict['quantity'] = 1
+        quantity_unit_dict['unit'] = quantity_unit_string
+    else:
+        quantity_unit_dict['quantity'] = int(quantity_unit_string[:i])
+        quantity_unit_dict['unit'] = quantity_unit_string[i:]
+    return quantity_unit_dict


Returning a dict is a bit overcomplicated, why just not returning a simple tuple?

Thank you so much for your feedback !
I have updated it.

rest_framework/throttling.py

Co-authored-by: Devid <[email protected]>

PravinKamble123 · 2023-08-21T15:37:50Z

This could be an interesting feature. However the code must be cleaned and also docs and example must be written for this.

I would really appreciate if you could help me and guide me further.

…b.com/PravinKamble123/django-rest-framework into feature/flexible-throttling-intervals

sevdog

This still requires to be documented in throttling.md.

Before merging it would be good to squash (rebase) this branch into a single commit to keep the history as clean as possible.

sevdog · 2023-08-22T15:47:19Z

rest_framework/utils/throttling_duration_parser.py

The point is that this method does not have any other use case, so IMO having an extra file for this does not seem good choice.

It could make sense if there was the need to have an utility method which needs to be used by throttling package and by others, as of new there is no need to have this in an extra package.

sevdog · 2023-08-22T15:53:08Z

rest_framework/utils/throttling_duration_parser.py

+    Args:
+        quantity_unit_string (str): A string that combines a numeric quantity and a unit, e.g., "5min", "10h".
+
+    Returns:
+        dict: A dictionary containing the parsed quantity and unit, with keys 'quantity' and 'unit'.
+              If the input string contains only a unit (e.g., "m"), quantity will be set to 1.


This does not match any other docstring style in the project, it will just make confusion.

I did not understand your point. Is it too much text ? or too long lines ?

To put it simple: the part with Args: and Returns: are not used by any other comment in this repository. Keeping that here may cause confusion.

PravinKamble123 · 2023-08-29T04:04:13Z

@sevdog @robhudson @roncohen need suggestion

sevdog · 2023-08-29T09:36:46Z

@PravinKamble123 wait for others to review this PR (please do not change what is done by maintainers of this repo). I already have done some checks on this PR, now you need to wait for furhter review and approval.

PravinKamble123 · 2023-09-17T15:19:00Z

@PravinKamble123 wait for others to review this PR (please do not change what is done by maintainers of this repo). I already have done some checks on this PR, now you need to wait for furhter review and approval.

@sevdog ok.

This commit introduces the ability to set custom time intervals for throttling, allowing users to specify intervals like per 5 minutes, per 2 hours, and per 5 days. This enhancement provides more flexibility in controlling request rates.

…b.com/PravinKamble123/django-rest-framework into feature/flexible-throttling-intervals

PravinKamble123 · 2023-11-02T08:19:26Z

@sevdog hi,
I have clear the unnecessary history of my branch.
I have squashed to single commit.

PravinKamble123 · 2023-11-02T08:30:27Z

@auvipy need feedback

sevdog · 2023-11-02T08:48:21Z

@PravinKamble123 You need to force push the squashed commit, as of now it is not available on github.

PravinKamble123 · 2023-11-02T09:01:30Z

@PravinKamble123 You need to force push the squashed commit, as of now it is not available on github.

@sevdog noted.

tomchristie · 2024-01-04T12:39:17Z

Appreciate the input, tho... https://github.com/encode/django-rest-framework/blob/master/CONTRIBUTING.md

At this point in its lifespan we consider Django REST framework to be essentially feature-complete. We may accept pull requests that track the continued development of Django versions, but would prefer not to accept new features or code formatting changes.

PravinKamble123 · 2024-01-04T12:47:08Z

No worries. I think was very closed. Eveything was tested. Its was not affecting the other functionality. Instead it was improving the functionality. And it is reviewed and appreciated by one the contributor. No problem. Thanks

…

On Thu, 4 Jan, 2024, 6:09 pm Tom Christie, ***@***.***> wrote: Closed #9079 <#9079>. — Reply to this email directly, view it on GitHub <#9079 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AVXHOJFN7PU7M4ORQN7CPC3YM2PIFAVCNFSM6AAAAAA3XNQRW2VHI2DSMVQWIX3LMV45UABCJFZXG5LFIV3GK3TUJZXXI2LGNFRWC5DJN5XDWMJRGM4DSNBSG4ZTQNY> . You are receiving this because you were mentioned.Message ID: ***@***.*** com>

tomchristie · 2024-01-04T13:09:24Z

Valid points, yep.
Perhaps if REST framework moves under alternative maintainer-ship the CONTRIBUTING.md policy might change.
(Or perhaps not?)

PravinKamble123 · 2024-09-04T05:29:31Z

If I want contribute again what should I have to do then

…

On Thu, 4 Jan, 2024, 6:39 pm Tom Christie, ***@***.***> wrote: Valid points, yep. Perhaps if REST framework moves under alternative maintainer-ship the CONTRIBUTING.md policy might change. (Or perhaps not?) — Reply to this email directly, view it on GitHub <#9079 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AVXHOJA7DMR7MQA6NVQMUW3YM2SZBAVCNFSM6AAAAAA3XNQRW2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNZXGA3TAOJZGY> . You are receiving this because you were mentioned.Message ID: ***@***.***>

PravinKamble123 · 2024-09-04T05:31:16Z

I want contribute to drf again with same enhancement. This enhancement will make our current app more robust.

…

On Wed, 4 Sept, 2024, 10:59 am pravin kamble, ***@***.***> wrote: If I want contribute again what should I have to do then On Thu, 4 Jan, 2024, 6:39 pm Tom Christie, ***@***.***> wrote: > Valid points, yep. > Perhaps if REST framework moves under alternative maintainer-ship the > CONTRIBUTING.md policy might change. > (Or perhaps not?) > > — > Reply to this email directly, view it on GitHub > <#9079 (comment)>, > or unsubscribe > <https://github.com/notifications/unsubscribe-auth/AVXHOJA7DMR7MQA6NVQMUW3YM2SZBAVCNFSM6AAAAAA3XNQRW2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNZXGA3TAOJZGY> > . > You are receiving this because you were mentioned.Message ID: > ***@***.***> >

auvipy · 2024-09-04T08:51:13Z

may be creating an extension would be better for a while for new features or contribute to drf-extensions

PravinKamble123 · 2024-09-04T10:06:35Z

Thanks for feedback. I have written only function. And I have tested it. I won't impact the existing features. This is just small enhancement I would say. What if I create new fresh branch from master and do same changes?. I respect all of your decisions but If you allow this would be my first contribution to drf.

…

On Wed, 4 Sept, 2024, 2:21 pm Asif Saif Uddin, ***@***.***> wrote: may be creating an extension would be better for a while for new features or contribute to drf-extensions — Reply to this email directly, view it on GitHub <#9079 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AVXHOJBLSZX2MGDRDSRUMDTZU3CZTAVCNFSM6AAAAABNTQHVLKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGMRYGI4DIMZWGU> . You are receiving this because you were mentioned.Message ID: ***@***.***>

tomchristie · 2024-09-04T12:18:24Z

At this point in its lifespan we consider Django REST framework to be essentially feature-complete. We may accept pull requests that track the continued development of Django versions, but would prefer not to accept new features or code formatting changes.

PravinKamble123 added 5 commits August 20, 2023 23:21

adding tests

71289ff

adding tests

decd4b8

fixing the whitespaces

c10c92c

PravinKamble123 closed this Aug 20, 2023

PravinKamble123 reopened this Aug 20, 2023

PravinKamble123 added 2 commits August 21, 2023 00:27

fixing the whitespaces

9becb0e

fixing the api view

e4a8a1e

sevdog suggested changes Aug 21, 2023

View reviewed changes

Update rest_framework/throttling.py

5d684b7

Co-authored-by: Devid <[email protected]>

PravinKamble123 added 5 commits August 21, 2023 21:41

update the throtting_duration_parser.py returning tuple instead of dict

f62927c

Merge branch 'feature/flexible-throttling-intervals' of https://githu…

97af799

…b.com/PravinKamble123/django-rest-framework into feature/flexible-throttling-intervals

update the tests

fea49b7

fix the white spacing

436f61c

fix the white spacing

6844bc2

PravinKamble123 requested a review from sevdog August 21, 2023 18:06

sevdog suggested changes Aug 22, 2023

View reviewed changes

PravinKamble123 added 3 commits August 22, 2023 22:28

update the docs with examples

8a77717

updating the docs and removing the extra file from package

4d54da3

updating the docs and removing the extra file from package

a5aa48b

PravinKamble123 requested a review from sevdog August 23, 2023 15:50

auvipy requested review from a team and removed request for sevdog August 24, 2023 06:59

PravinKamble123 requested a review from sevdog August 24, 2023 15:08

sevdog mentioned this pull request Sep 5, 2023

Add support for denominators to throttle strings #9089

Closed

auvipy removed the request for review from sevdog September 9, 2023 08:27

auvipy self-requested a review September 9, 2023 08:27

PravinKamble123 requested a review from sevdog October 14, 2023 03:54

PravinKamble123 added 2 commits October 14, 2023 10:48

Merge branch 'feature/flexible-throttling-intervals' of https://githu…

035d98e

…b.com/PravinKamble123/django-rest-framework into feature/flexible-throttling-intervals

Fixed the intendtation

2b67e06

tomchristie closed this Jan 4, 2024

encode locked as resolved and limited conversation to collaborators Sep 4, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add support for flexible throttling intervals #9079

feat: Add support for flexible throttling intervals #9079

PravinKamble123 commented Aug 20, 2023

sevdog left a comment

sevdog Aug 21, 2023

PravinKamble123 Aug 21, 2023

sevdog Aug 22, 2023

PravinKamble123 Aug 22, 2023

sevdog Aug 21, 2023

PravinKamble123 Aug 21, 2023 •

edited

Loading

PravinKamble123 commented Aug 21, 2023

sevdog left a comment

sevdog Aug 22, 2023

sevdog Aug 22, 2023

PravinKamble123 Aug 22, 2023

sevdog Aug 24, 2023

PravinKamble123 commented Aug 29, 2023

sevdog commented Aug 29, 2023

PravinKamble123 commented Sep 17, 2023

PravinKamble123 commented Nov 2, 2023

PravinKamble123 commented Nov 2, 2023

sevdog commented Nov 2, 2023

PravinKamble123 commented Nov 2, 2023

tomchristie commented Jan 4, 2024

PravinKamble123 commented Jan 4, 2024 via email

tomchristie commented Jan 4, 2024

PravinKamble123 commented Sep 4, 2024 via email

PravinKamble123 commented Sep 4, 2024 via email

auvipy commented Sep 4, 2024

PravinKamble123 commented Sep 4, 2024 via email

tomchristie commented Sep 4, 2024

feat: Add support for flexible throttling intervals #9079

feat: Add support for flexible throttling intervals #9079

Conversation

PravinKamble123 commented Aug 20, 2023

Description

sevdog left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PravinKamble123 Aug 21, 2023 • edited Loading

Choose a reason for hiding this comment

PravinKamble123 commented Aug 21, 2023

sevdog left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PravinKamble123 commented Aug 29, 2023

sevdog commented Aug 29, 2023

PravinKamble123 commented Sep 17, 2023

PravinKamble123 commented Nov 2, 2023

PravinKamble123 commented Nov 2, 2023

sevdog commented Nov 2, 2023

PravinKamble123 commented Nov 2, 2023

tomchristie commented Jan 4, 2024

PravinKamble123 commented Jan 4, 2024 via email

tomchristie commented Jan 4, 2024

PravinKamble123 commented Sep 4, 2024 via email

PravinKamble123 commented Sep 4, 2024 via email

auvipy commented Sep 4, 2024

PravinKamble123 commented Sep 4, 2024 via email

tomchristie commented Sep 4, 2024

PravinKamble123 Aug 21, 2023 •

edited

Loading