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

Pricing improvements and bug fixes #379

Merged
merged 31 commits into from
Jul 14, 2024
Merged

Pricing improvements and bug fixes #379

merged 31 commits into from
Jul 14, 2024

Conversation

nikochiko
Copy link
Member

@nikochiko nikochiko commented Jun 10, 2024

Bug fixes:

  • ux: fix plan upgrade/downgrade button by setting unique key for each st.button(...)
  • webhook handling: don't delete subscriptions upon cancellation event
  • webhook handling: ignore cancellation event on subscriptions not in the database
  • webhook handling: use .select_for_update() to update subscriptions and avoid race condition with concurrent threads

Refactors

  • webhook handling: refactor for clarity, handling same event multiple times, and for reuse between stripe/paypal code

Improvements

  • ux: use st.run_in_thread(...) on paypal's payment_processing page
  • logging: add sentry-sdk loguru extension to send logging events to sentry. to silently ignore but still capture errors to sentry, this lets us just do logger.exception(...) or logger.critical(...) and works on both the local and production environments. (all these exceptions will be raised in sentry the same as sentry_sdk.capture_exception)
  • auto-recharge: move background task trigger to after a run has completed
  • auto-recharge: auto-recharge before starting run if user has insufficient balance

Commits:

  • Don't make use of transaction.atomic in handle_subscription_updated
  • paypal.Subscription.cancel: ignore when sub is already cancelled / expired
  • Fix paypal next invoice time when it is None
  • Payment processing page: run paypal subscription update in thread
  • sentry_sdk: upgrade to 1.45, add loguru extra to capture logged info
  • Refactor auto-recharge functionality with exceptions
  • Refactor payments webhook handling code into payments/webhooks.py
  • Add BasePage.run_with_auto_recharge
  • recipe runner task: s/page.run/page.run_with_auto_recharge
  • api: don't auto-recharge before the run (we do that afterwards)
  • Remove subscription-change logic from routers/account.py
  • billing_page: fix same-key bug in buttons, refactor for clarity
  • Fix Subscription model: allow null-values for auto-recharge config in DB
  • Add missing migration for blank=True on AppUser.subscription

Q/A checklist

  • If you add new dependencies, did you update the lock file?
poetry lock --no-update
  • Run tests
ulimit -n unlimited && ./scripts/run-tests.sh
  • Do a self code review of the changes - Read the diff at least twice.
  • Carefully think about the stuff that might break because of this change - this sounds obvious but it's easy to forget to do "Go to references" on each function you're changing and see if it's used in a way you didn't expect.
  • The relevant pages still run when you press submit
  • The API for those pages still work (API tab)
  • The public API interface doesn't change if you didn't want it to (check API tab > docs page)
  • Do your UI changes (if applicable) look acceptable on mobile?
  • Ensure you have not regressed the import time unless you have a good reason to do so.
    You can visualize this using tuna:
python3 -X importtime -c 'import server' 2> out.log && tuna out.log

To measure import time for a specific library:

$ time python -c 'import pandas'

________________________________________________________
Executed in    1.15 secs    fish           external
   usr time    2.22 secs   86.00 micros    2.22 secs
   sys time    0.72 secs  613.00 micros    0.72 secs

To reduce import times, import libraries that take a long time inside the functions that use them instead of at the top of the file:

def my_function():
    import pandas as pd
    ...

migrations.AlterField(
model_name='appuser',
name='subscription',
field=models.OneToOneField(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='user', to='payments.subscription'),
Copy link
Member Author

Choose a reason for hiding this comment

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

this migration is not from a model change in another commit.

the addition is blank=True

run_low_balance_email_check(uid)
run_auto_recharge_gracefully(uid)
Copy link
Member Author

Choose a reason for hiding this comment

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

this is run in the same thread, at the end of a task. not in a background task

Comment on lines 2008 to 2010
if not settings.CREDITS_TO_DEDUCT_PER_RUN:
return True

Copy link
Member Author

Choose a reason for hiding this comment

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

check_credits() was always being used with this check.

i felt more convenient to include that check here so we could call check_credits() only whenever we wanted.

Comment on lines +152 to +154
_render_plan_action_button(
user, plan, current_plan, selected_payment_provider
)
Copy link
Member Author

Choose a reason for hiding this comment

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

payment_provider name became ambiguous in the later functions, where it wasn't obvious that payment_provider is the one selected by user in the UI (from the radio button)

nikochiko and others added 8 commits June 10, 2024 18:21
Earlier, prorations were on, and someone who purchases a subscription
today will be charged the prorated amount + the next month's charge
in the next invoice.
This commit fixes that behaviour by charging the full subscription
amount on the same day. The billing cycle will also change to create
the next invoice 1 month from the same day.
…ents

# Conflicts:
#	app_users/migrations/0017_alter_appuser_subscription.py
#	celeryapp/tasks.py
#	daras_ai_v2/base.py
#	poetry.lock
#	routers/api.py
optimize run time of runner_task: check for credits only once in entire call chain, separate post run stuff into a separate task
fix run complete email for empty prompts
@devxpy devxpy force-pushed the pricing-v2-improvements branch from d42e719 to e6b9955 Compare July 14, 2024 20:58
@devxpy devxpy merged commit 0146f0c into master Jul 14, 2024
1 check passed
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.

2 participants