Skip to content

Conversation

@emma-sg
Copy link
Member

@emma-sg emma-sg commented Oct 28, 2025

@emma-sg emma-sg marked this pull request as draft October 28, 2025 22:45
@emma-sg emma-sg force-pushed the additional-minutes-addon-purchase branch 2 times, most recently from 4ee2020 to 82a6295 Compare November 4, 2025 23:40
@emma-sg emma-sg force-pushed the additional-minutes-addon-purchase branch from 5ad2648 to 93fd027 Compare November 10, 2025 22:31
@emma-sg emma-sg force-pushed the additional-minutes-addon-purchase branch 4 times, most recently from e583fe7 to 9fa5509 Compare November 18, 2025 20:12
@emma-sg emma-sg marked this pull request as ready for review December 4, 2025 18:58
@ikreymer ikreymer requested a review from tw4l December 4, 2025 19:17
@socket-security

This comment was marked as resolved.

@tw4l
Copy link
Member

tw4l commented Dec 5, 2025

Working well on dev, with a really smooth user experience. I left a few comments. I am leaning with Emma toward handling the org quota update in a transaction as the right approach here, as long as the effort isn't prohibitive (in which case #2999 seems like a good option to tide us over for now, but maybe with some more comments to help devs reading the code walk through the business logic of the aggregation).

It might also be good to use this opportunity to improve our test coverage for the update quota method a little bit regardless of which path we take to test edge cases like attempting to set a quota below 0 in an update.

- add get_checkout_url method to create checkout session for additional
minutes - add /checkout/execution-minutes endpoint to handle addon
purchase requests (this will need to be updated in dev/prod playbooks) -
update CI workflows to remove portalUrl suffix from secret value - add
test endpoint in echo server to mock checkout response
@emma-sg emma-sg force-pushed the additional-minutes-addon-purchase branch from 261f9b8 to 43bb583 Compare December 9, 2025 23:09
emma-sg and others added 6 commits December 9, 2025 18:16
To be merged into #2944 

Adds a causally consistent session specifically to the quota update
method so that its three operations can operate more-or-less atomically.

We'd looked at doing this with a transaction, but they require a replica
set or sharding, which isn't always included in self-hosted Browsertrix
deployments. For now this should improve reliability for the
multi-operation `update_quotas` method, even if it's not perfectly
atomic — it's pretty unlikely at the scale we're working with here for
there to be multiple simultaneous operations for a single org.

Tested with local deployment.

---------

Co-authored-by: Ilya Kreymer <[email protected]>
as discussed in discord
Copy link
Member

@ikreymer ikreymer left a comment

Choose a reason for hiding this comment

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

Nice work! Tested extensively on dev and working as expected. Nice improvements to the meter rendering as well.

Copy link
Member

@ikreymer ikreymer left a comment

Choose a reason for hiding this comment

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

Nice work! Tested extensively on dev with test accounts, working as expected, and new meter rendering is a significant improvement as well!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: In Review

Development

Successfully merging this pull request may close these issues.

[Feature]: Support purchasing of additional crawling minutes

4 participants