Fix missing ds error

[Charl1996]

Technical Summary

Ticket

An error was picked up by sentry in which a case made changes to a UCR that does not exist. This PR is simply to check whether the table exists before making the change. If the table does not exist, no work is necessary.

That said, I don't think this addresses the real problem, i.e. why the case change is propagated for this config which is supposedly deleted. This issue is to be investigated further, but this PR change will at least silence the error.

Feature Flag

UCRs

Safety Assurance

Safety story

--

Automated test coverage

No tests, don't think it's necessary for such a simple change.

QA Plan

No QA

Rollback instructions

• This PR can be reverted after deploy with no further considerations

Labels & Review

• Risk label is set correctly
• The set of people pinged as reviewers is appropriate for the level of risk of the change

[gherceg]

I think this was mentioned in slack already, but just want to confirm that while this is the short term solution, there is also an ongoing effort to understand how we are getting to the point where we are trying to update rows on a deleted UCR?

[Charl1996]

I think this was mentioned in slack already, but just want to confirm that while this is the short term solution, there is also an ongoing effort to understand how we are getting to the point where we are trying to update rows on a deleted UCR?

Correct.

[gherceg]

  FAIL: corehq.apps.userreports.tests.test_save_errors:SaveErrorsTest.test_raise_error_for_missing_table
  ----------------------------------------------------------------------
  Traceback (most recent call last):
    File "/mnt/commcare-hq-ro/corehq/apps/userreports/tests/test_save_errors.py", line 54, in test_raise_error_for_missing_table
      self.adapter.best_effort_save(doc)
  AssertionError: TableNotFoundWarning not raised

Test failure looks related.

[Charl1996]

Turns out we expect it to fail if this condition is not met, so removing this piece.

[mkangia]

Hey @Charl1996

4 commits and 1 file changed caught me by surprise, can we please clean up to just keep the relevant commit(s)?

Also, if we are firing a query to check for table each time we delete, should we rather delete happen and catch the exception instead?

[mkangia]

Does this fire a new query or it already is aware?

[Charl1996]

Good question.

The table_exists method invokes the has_table method on the engine object. The engine is created here. Looking manually through the sqlalchemy code it looks like it uses cached info.

I'm just wondering now, if it's using cached info, what's the chance of it getting out of date? I'll try and see if I can find out more about how this works.

[Charl1996]

@mkangia Ok, I can't find good documentation on how the caching updates work, so the safest course is probably to just catch the exception, since we're making a DB request in any case.

[Charl1996]

Here's a draft PR

[Charl1996]

Closed in favour of #34595