-
Notifications
You must be signed in to change notification settings - Fork 16
feat: terminate stuck paid checkout intent #913
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
feat: terminate stuck paid checkout intent #913
Conversation
| self.stdout.write( | ||
| f'{mode_label}Checking for CheckoutIntent records stuck in paid state ' | ||
| f'for more than {threshold_seconds} seconds...' | ||
| ) | ||
| logger.info( | ||
| '%sStarting mark_stalled_checkout_intents command with threshold=%s seconds', | ||
| mode_label, | ||
| threshold_seconds, | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can remove one of these duplicative messages throughout the code. I'm pretty sure logger already goes to stdout, and the stdout messages are not already designed to be machine-readable, so I think we can just remove the stdout.write() lines and keep the logger.info lines
Functionally, you're using stdout.write() as a logger, but it's weaker than logger because it lacks timestamps and other useful metadata.
| self.stdout.write( | ||
| self.style.SUCCESS( | ||
| f'{mode_label}Command completed successfully' | ||
| ) | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO, this is the only self.stdout.write() I'd keep, to leverage the success style.
| threshold_seconds, | ||
| ) | ||
|
|
||
| if dry_run: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This implementation of a dry run isn't quite appropriate because it replaces too much business logic. A better approach would be to pass a dry run flag into a lower level function and implement dry run within.
| updated_count, updated_uuids = CheckoutIntent.mark_stalled_fulfillment_intents( | ||
| stalled_threshold_seconds=threshold_seconds | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is probably where dry_run needs to be passed.
Description:
Add a description of your changes here.
Jira:
ENT-11059 & ENT-11123
Merge checklist:
./manage.py makemigrationshas been runPost merge: