-
-
Notifications
You must be signed in to change notification settings - Fork 214
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
Retry 400 "Bad Request" responses #35165
Conversation
@contextmanager | ||
def repeat_record(self): | ||
repeat_record = self.repeater.repeat_records.create( | ||
domain=DOMAIN, | ||
payload_id='eggs', | ||
registered_at=timezone.now(), | ||
) | ||
try: | ||
yield repeat_record | ||
finally: | ||
repeat_record.delete() |
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.
We don't actually have to handle cleanup for a SQL Table in a TestCase
class since all SQL updates happen within a transaction that is rolled back at the end of the test right? Could this be simplified to not use a context manager or is there a benefit to this I'm missing?
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.
It could totally just create the repeat record. I'll change it.
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.
The failing test looks related |
Technical Summary
Context: SAAS-15905 discussion
Currently when Data Forwarding gets a 400 "Bad Request" response, it handles the repeat record as an invalid payload.
However, when Data Forwarding forwards a case update for a case that has not yet been created, CommCare's Configurable API returns a 400 "Bad Request" response, and we want that repeat record to be retried, so that the next attempt might be after the case has been created.
This change is to retry 400 "Bad Request" responses. It feels arguably justifiable that the 400 error code is broad enough to include situations where one might want to retry the request.
Safety Assurance
Safety story
Automated test coverage
Includes tests
Rollback instructions
Labels & Review