New execution policy that handles both REST and GraphQL limits

[clement911]

Here is the new execution policy that handles both REST and GraphQL limits.

• It's called LeakyBucketExecutionPolicy and implements logic described at https://shopify.dev/api/usage/rate-limits
• I've marked the SmartRetryExecutionPolicy as obsolete because I expect this one to be used instead.
• Most of the files changed are the test files, which I've updated to use this new policy
• I provided several tests for the leaky bucket algorithm
• Given it's a new policy, there should be no breaking changes
• Except that the GraphService now takes an extra (optional) parameter to indicate the query cost. Given that this policy tracks the current bucket level, it can tell whether a query will be throttled by looking at the expected query cost.

This is just a new policy for rate limits.
In the future I'd like to improve the GraphService to support variables and typed responses, but I don't want to change too much at a time.

@nozzlegear let me know if you have any questions.

[nozzlegear]

Ah this is awesome, thanks for throwing this together so quickly! I'm going to review it and test the new policy out, and then I'll let you know if I have any feedback (although it looks excellent already).

My only "concern" is marking the previous policy as obsolete even though this one is meant to supersede it. That tends to cause developers to send me grumpy emails about the warnings their compilers are suddenly spitting out at them. If it's a drop-in replacement for the policy though, that's really not asking that much and we have to be able to deprecate code at some point.

[clement911]

Sure, I've been wanting to do this for a while and thinking about it.
I definitely did some thorough automated and manual testing but I'd still love to hear any feedback you have.

Regarding the deprecation marker of the previous policy, I did hesitate on this. Feel free to remove it.
The new policy can do everything the old one did, and more (GraphQL) so in the long term, I don't see a use case for it but we can deprecate it later...

[clement911]

Hi @nozzlegear , I've removed the [Obsolete] attribute on the SmartRetryExecutionPolicy.
Is there anything else needed before you can merge?

[nozzlegear]

This is going to sound silly, but now I'm thinking we should use the [Obsolete] attribute! 😅 I'm not sure when a 6.0 release will happen (I'm starting to gear up for it), but it'd be nice to have the warning in there so everyone can prepare and get swapped over before it's removed.

…

-- Josh Harms

On Mon, Sep 20, 2021, at 03:53, Clement Gutel wrote: Hi @nozzlegear <https://github.com/nozzlegear> , I've removed the [Obsolete] attribute on the SmartRetryExecutionPolicy. Is there anything else needed before you can merge? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#667 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AASOE7AQ3U2LZKSYJRKPC6LUC3ZA3ANCNFSM5ECRXVAQ>.

[clement911]

Done.
@nozzlegear , please let me know if you're not able to merge in the next couple of days as I might have to create a custom build otherwise.

Regarding 6.0, what do you have in mind for this release?

[nozzlegear]

Excellent, thank you! I've merged it and will publish either this evening or tomorrow morning. In regards to 6.0, the biggest thing I want to tackle is partial object updating. The package has been sorely missing that and it's the source of many questions/issues. I'm open to additional features/changes for 6.0 as well if you've got any ideas!

[clement911]

Sorry to bug you again but it would be great it you could publish. I'd like to avoid creating a custom fork build.

For partial updates, I think #642 (comment) is still my favored option at this point.

[nozzlegear]

@clement911 Woops, thanks for the reminder! You're not bugging me, please do poke me if I say I'm going to publish something and then forget to publish it. 😜 I've published this in 5.13.0, thank you very much for the work you've done on this new policy!

[clement911]

Thanks!