Merge pull request #33729 from dimagi/mjr/user_importer_critical_section

Enable 'fail_hard' for Bulk User Uploads

corehq/apps/user_importer/importer.py

@@ -3,6 +3,7 @@
 import random
 from collections import defaultdict, namedtuple
 from datetime import datetime
+from corehq.util.soft_assert.api import soft_assert
 
 from django.db import DEFAULT_DB_ALIAS
 
@@ -52,6 +53,8 @@
 from corehq.toggles import DOMAIN_PERMISSIONS_MIRROR, TABLEAU_USER_SYNCING
 from corehq.apps.sms.util import validate_phone_number
 
+from dimagi.utils.logging import notify_error
+
 required_headers = set(['username'])
 web_required_headers = set(['username', 'role'])
 allowed_headers = set([
@@ -603,7 +606,21 @@ def create_or_update_commcare_users_and_groups(upload_domain, user_specs, upload
             if web_user_username:
                 user.get_user_data(domain)['login_as_user'] = web_user_username
 
-            user.save()
+            try:
+                user.save(fail_hard=True)
+            except Exception as e:
+                # HACK: Catching all exception here is temporary. We believe that user critical sections
+                # are not behaving properly, and this catch-all is here to identify the problem
+                status_row['flag'] = str(e)
+                notify_error(f'Error while processing bulk import: {str(e)}')
+                soft_assert(to='{}@{}'.format('mriley', 'dimagi.com'), send_to_ops=False)(
+                    False,
+                    'Error while processing bulk import',
+                    e
+                )
+                ret["rows"].append(status_row)
+                continue
+
             log = commcare_user_importer.save_log()
 
             if web_user_username:

corehq/apps/users/models.py

@@ -1468,13 +1468,18 @@ def save_docs(cls, docs, **kwargs):
 
     bulk_save = save_docs
 
-    def save(self, fire_signals=True, update_django_user=True, **params):
+    def save(self, fire_signals=True, update_django_user=True, fail_hard=False, **params):
+        # fail_hard determines whether the save should fail if it cannot obtain the critical section
+        # historically, the critical section hasn't been enforced, but enforcing it is a dramatic change
+        # for our system. The goal here is to allow the programmer to specify fail_hard on a workflow-by-workflow
+        # basis, so we can gradually shift to all saves requiring the critical section.
+
         # HEADS UP!
         # When updating this method, please also ensure that your updates also
         # carry over to bulk_auto_deactivate_commcare_users.
         self.last_modified = datetime.utcnow()
         self.clear_quickcache_for_user()
-        with CriticalSection(['username-check-%s' % self.username], timeout=120):
+        with CriticalSection(['username-check-%s' % self.username], fail_hard=fail_hard, timeout=120):
             # test no username conflict
             by_username = self.get_db().view('users/by_username', key=self.username, reduce=False).first()
             if by_username and by_username['id'] != self._id: