Skip to content
This repository has been archived by the owner on Feb 8, 2018. It is now read-only.

Throttle better #4349

Merged
merged 2 commits into from
Mar 17, 2017
Merged

Throttle better #4349

merged 2 commits into from
Mar 17, 2017

Conversation

chadwhitacre
Copy link
Contributor

@chadwhitacre chadwhitacre commented Feb 22, 2017

Our throttling algorithm is naive. Now that we have an isolated queue_email path (#4346), we should use that as a choke-point for throttling for all user-initiated email.

https://hackerone.com/reports/108645

@chadwhitacre chadwhitacre changed the base branch from master to simplify-verify February 22, 2017 22:45
@chadwhitacre
Copy link
Contributor Author

chadwhitacre commented Feb 22, 2017

This is somewhat complicated by the fact that we don't want system-generated emails to affect throttling, only user-initiated messages.

@chadwhitacre
Copy link
Contributor Author

chadwhitacre commented Feb 27, 2017

Alright! All caught up after making upstream changes (see chain listed under "new email subflooring" pre-req on #4305). Ready to make progress here again, I think ...

@mattbk
Copy link
Contributor

mattbk commented Feb 28, 2017

!m @whit537!

@chadwhitacre
Copy link
Contributor Author

chadwhitacre commented Mar 2, 2017

Not sure the new tests aren't raising ...

Typo in ea2c77f. 😊

@chadwhitacre
Copy link
Contributor Author

Okay! With a204a43, the email refactoring and bugfix chain is complete at a first pass. We've already started merging w/ #4343 and #4344, and #4345 is on deck now. Since we seem to be moving along at a good clip, I'm going to hold off on rebasing the whole chain right now.

@EdOverflow
Copy link
Contributor

This HackerOne report is related: h1:212880.

@chadwhitacre
Copy link
Contributor Author

Rebased onto master post-#4348, was a204a43.

@chadwhitacre chadwhitacre changed the base branch from simplify-verify to master March 17, 2017 12:04
@chadwhitacre chadwhitacre merged commit b7449e3 into master Mar 17, 2017
@chadwhitacre
Copy link
Contributor Author

Self-merged per #4305 (comment) and #4360 (comment).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants