Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Throttle iteration API #283

Merged
merged 4 commits into from
Jul 29, 2022
Merged

Throttle iteration API #283

merged 4 commits into from
Jul 29, 2022

Conversation

Pimm
Copy link
Collaborator

@Pimm Pimm commented Jul 20, 2022

As described in this issue, developers should not request large sets of data from the Mollie API in most scenarios. This PR aims to discourage doing so, by introducing throttling.

If a developer (and thereby a merchant) requests a large set of data, the throttling will spread out the pressure on the Mollie API over a larger period of time. But additionally, it might send a signal to the developer, asking "hey, this thing you're doing, it's costly, is there no better way?"

(Closes #271)

Interface

In this implementation, the developer is in control of the throttling:

for await (let payment of mollieClient.payments.iterate({ valuesPerMinute: 5000 })) {
  []
}

Developers are using this client voluntarily. They can opt-out of using it at any time, and just use curl to communicate with the Mollie API instead. This is why I feel "forcing" a certain amount of throttling is not productive. If a developer chooses to set the valuesPerMinute to Number.POSITIVE_INFINITY, that's a conscious decision on their part.

(valuesPerMinute is optional, making this change backward compatible.)

Better

A safer form of throttling ‒ in the sense that it's more difficult for developers to circumvent ‒ would be one that is applied on the Mollie API itself rather than in the client(s). When such throttling is introduced, it can be removed from the client.

Best

Neither spreading the requests over a longer period of time, nor asking the developer to find a way to make fewer request is an ideal solution. The real solution is making more efficient requests, by supporting filtering and searching in the Mollie API itself.

Once the Mollie API supports filtering payments by currency, for instance, we can expand the iteration API:

// Find the 10 latest euro payments.
const iterator = mollieClient.payments.iterate()
	.filter({ currency: 'EUR' })
	.take(10);
for await (let payment of iterator) {
	// Apply logic.
}

This would guarantee one single call to the Mollie API (v2/payments?currency=EUR&limit=10).

src/communication/NetworkClient.ts Outdated Show resolved Hide resolved
src/communication/NetworkClient.ts Show resolved Hide resolved
@Pimm Pimm force-pushed the pimm/throttle branch 2 times, most recently from 7763bae to c097683 Compare July 22, 2022 11:17
Copy link
Collaborator

@edorivai edorivai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left one comment on testing, but otherwise: LGTM 🚀

Pimm added 3 commits July 28, 2022 13:01
Instead of requesting three pages with 200 values if 600 values are demanded, the iteration API now requests two pages with 250 values and one with 100 values.
The iterate methods ‒ by default ‒ produce 500 values per minute. This hopefully discourages developers from using this API to request an unreasonable amount of values (payments/customers/…). See: #271
…ute.

Additionally, the throttling is now done in increments of six seconds, rather than minutes. This results in "smoother" throttling.
@Pimm Pimm merged commit 559dcba into master Jul 29, 2022
@Pimm Pimm deleted the pimm/throttle branch July 29, 2022 08:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Discourage developers from requesting large sets of data
3 participants