feat(charging): Support the idempotency_key param in Actor.charge() - #1123
feat(charging): Support the idempotency_key param in Actor.charge()#1123Mantisus wants to merge 8 commits into
idempotency_key param in Actor.charge()#1123Conversation
Mantisus
changed the title
idempotency_key param in Actor.charge()
| # Only remember a key that stands for a charge the platform actually received. Off the platform there | ||
| # is no request at all and the registry is the only thing providing deduplication. | ||
| if idempotency_key is not None and (not self._is_at_home or charge_sent): | ||
| # Remember the key for every charge that was counted, including events the API never receives, such as | ||
| # synthetic and tier-priced ones - those are counted locally and a repeat would count them twice. | ||
| if idempotency_key is not None: |
There was a problem hiding this comment.
With this update, the idempotency_key will be saved even for events that do not exist.
|
@barjin, Please take a look at the implementation. In the TS SDK, the |
barjin
left a comment
barjin
left a comment
There was a problem hiding this comment.
Thank you @Mantisus !
Good catch, we need this in JS SDK as well, otherwise the deduplicated charges will eventually desync the running total count. I opened an issue here.
Otherwise, I left a few ideas regarding the implementation ⬇️ Thanks again!
| # A repeat is resolved from this registry rather than left to the platform, whose own idempotency | ||
| # record expires after a few minutes: a late repeat would charge a second time, and counting it here | ||
| # would inflate the charging state and make the run hit `max_total_charge_usd` early. |
There was a problem hiding this comment.
This is surprising, but true (the API implementation has a 3-minute expiration window on the idempotency records (source)).
I think we should follow the Platform behaviour here and allow for this idempotency "cooldown" period. This should also allow us to remove the old local idempotency records.
| Keys are remembered for the lifetime of the Actor process. A key belongs to a single event, so | ||
| reusing one for a different event raises `ValueError`, as does passing a blank key. |
There was a problem hiding this comment.
A key belongs to a single event, so reusing one for a different event raises
ValueError
This does sound logical, but it's not how the API works. Should we implement this logic here?
4 participants
Adds support for the
idempotency_keyparameter inActor.charge().Closes: #1122