Repository: SFDO-Tooling/CumulusCI
Pull request #3065 (2022-02-09 01:31:32)

Better error messages and warnings
# Changes

Nothing for release notes.

The following log messages were added. @pconrad-sfdo 

# Changed Texts

Querying for Account objects matching "name!=NULL"

Updating 3 Account records

## Changed Result messages

All records, no errors:

* All account objects successfully updated (3).

All records, 3 errors:

 * All account objects processed (3). 3 errors

With a filter and no errors discovered:

* Account objects matching "name!=null" successfully updated (3).

Or, with a filter and with errors:

* Account objects matching "name!=null" processed (3). 3 errors 



Patch:
diff --git a/cumulusci/tasks/bulkdata/update_data.py b/cumulusci/tasks/bulkdata/update_data.py
index fa90d7117..3786d1f14 100644
--- a/cumulusci/tasks/bulkdata/update_data.py
+++ b/cumulusci/tasks/bulkdata/update_data.py
@@ -118,13 +118,29 @@ class UpdateData(BaseSalesforceApiTask):
             return
         with self.save_records(qs, fields) as csvfile, TemporaryDirectory() as outdir:
             csv_out = self.generate_data(csvfile, Path(outdir))
-            with csv_out.open() as csv_out:
-                enriched_data = csv.DictReader(csv_out)
+
+            with csv_out.open() as csv_out_open:
+                enriched_data = csv.DictReader(csv_out_open)
                 oid_index = enriched_data.fieldnames.index("Oid")
                 enriched_data.fieldnames[oid_index] = "Id"
-                self.load_data(qs.job_result.records_processed, enriched_data)
+                ds = self.load_data(qs.job_result.records_processed, enriched_data)
+            records = ds.job_result.records_processed
+            errors = ds.job_result.total_row_errors
+
+        obj_description = self._object_description(obj).capitalize()
+        if errors:
+            self.logger.info(
+                f"{obj_description} processed ({records}). {errors} errors"
+            )
+        else:
+            self.logger.info(f"{obj_description} successfully updated ({records}).")
 
-        self.logger.info(f"Updated {self._object_description(obj)}")
+        if errors and not self.ignore_row_errors:
+            plural = "s" if errors > 1 else ""
+            raise BulkDataException(f"{errors} update error{plural}")
+        else:
+            self.return_values = {**ds.job_result.simplify()}
+        return self.return_values
 
     def generate_data(self, csvfile: Path, outdir: Path) -> Path:
         generate_data(
@@ -169,7 +185,7 @@ class UpdateData(BaseSalesforceApiTask):
             api=DataApi.SMART,  # maybe this should always be bulk
         )
 
-        self.logger.info(f"Querying for {obj} objects")
+        self.logger.info(f"Querying for {self._object_description(obj)}")
         qs.query()
         if qs.job_result.status is not DataOperationStatus.SUCCESS:
             raise BulkDataException(
@@ -181,7 +197,7 @@ class UpdateData(BaseSalesforceApiTask):
 
     def load_data(self, row_count: int, records: csv.DictReader):
         obj = self.sobject
-        self.logger.info(f"Loading data {self._object_description(obj)} ")
+        self.logger.info(f"Updating {row_count} {obj} records")
         fieldnames = [
             f for f in records.fieldnames if f != "id" and not f.startswith("_")
         ]
@@ -211,9 +227,10 @@ class UpdateData(BaseSalesforceApiTask):
                 f"Unable to update records for {obj}: {','.join(ds.job_result.job_errors)}"
             )
 
-        error_checker = RowErrorChecker(
-            self.logger, self.ignore_row_errors, self.row_warning_limit
-        )
+        # "ignore" row errors because at this point it's too late. We're done the
+        # job entirely. Raising an exception isn't very helpful. We can raise
+        # it later.
+        error_checker = RowErrorChecker(self.logger, True, self.row_warning_limit)
         for result in ds.get_results():
             error_checker.check_for_row_error(result, result.id)
         return ds
diff --git a/datasets/update.recipe.yml b/datasets/update.recipe.yml
index 52e99dc3e..4139a7fe1 100644
--- a/datasets/update.recipe.yml
+++ b/datasets/update.recipe.yml
@@ -1,7 +1,5 @@
 # cci task run update_data --recipe datasets/update.recipe.yml
 #                          --object Account --fields Name --org qa
-- plugin: snowfakery.standard_plugins.Counters
-
 - object: Account
   fields:
       BillingStreet:


Test patch:
diff --git a/cumulusci/tasks/bulkdata/tests/test_updates.py b/cumulusci/tasks/bulkdata/tests/test_updates.py
index 23713714a..8e3d583da 100644
--- a/cumulusci/tasks/bulkdata/tests/test_updates.py
+++ b/cumulusci/tasks/bulkdata/tests/test_updates.py
@@ -147,6 +147,15 @@ class FakeDMLOperationResult(FakeOperationResult):
     def end(self):
         pass
 
+    @property
+    def job_result(self):
+        return DataOperationJobResult(
+            self.results.job_result_status,
+            [],
+            len(self.results.results),
+            sum(1 for r in self.results.results if r.success is False),
+        )
+
 
 def _fake_val_and_inject_ns(
     should_inject_namespaces: bool,
@@ -274,7 +283,7 @@ class TestUpdates:
         bulkapi_responses.add_query_operation(
             sobject="Account",
             fields=["Id", "Name"],
-            query="SELECT Id,Name FROM Account WHERE Name Like '%ark%'",
+            query="SELECT Id,Name FROM Account WHERE name LIKE '%ark%'",
             api=DataApi.SMART,
             results=[
                 ["OID000BLAH", "Mark Benihoff"],
@@ -299,7 +308,7 @@ class TestUpdates:
                 "object": "Account",
                 "recipe": "datasets/update.recipe.yml",
                 "fields": ["Name"],
-                "where": "Name Like '%ark%'",
+                "where": "name LIKE '%ark%'",
             },
         )
         task._validate_and_inject_namespace_prefixes = _fake_val_and_inject_ns
@@ -307,7 +316,7 @@ class TestUpdates:
         task()
         last_message = task.logger.mock_calls[-1].args[0]
         assert last_message.startswith(
-            "Updated Account objects matching \"Name Like '%ark%'\""
+            "Account objects matching \"name like '%ark%'\""
         ), last_message
 
     @bulkapi_responses.activate
@@ -390,6 +399,97 @@ class TestUpdates:
             task()
         assert "Unable to update records" in str(e.value)
 
+    @bulkapi_responses.activate
+    def test_update_row_errors(self, create_task):
+        bulkapi_responses.add_query_operation(
+            sobject="Account",
+            fields=["Id", "Name"],
+            query="SELECT Id,Name FROM Account",
+            api=DataApi.SMART,
+            results=[
+                ["OID000BLAH", "Mark Benihoff"],
+                ["OID000BLAH2", "Parkour Hairish"],
+            ],
+        )
+        loader = mock.Mock()
+        bulkapi_responses.add_dml_operation(
+            sobject="Account",
+            operation=DataOperationType.UPDATE,
+            fields=["BillingStreet", "Description", "NumberOfEmployees", "Id"],
+            api_options={},
+            api=DataApi.SMART,
+            volume=2,
+            results=[
+                DataOperationResult("OID000BEEF", success=True, error=""),
+                DataOperationResult("OID000BLAH", success=False, error="humbug"),
+            ],
+            loader_callback=loader,
+            job_result_status=DataOperationStatus.ROW_FAILURE,
+        )
+
+        task = create_task(
+            UpdateData,
+            {
+                "object": "Account",
+                "recipe": "datasets/update.recipe.yml",
+                "fields": ["Name"],
+                "ignore_row_errors": True,
+            },
+        )
+
+        task._validate_and_inject_namespace_prefixes = _fake_val_and_inject_ns
+        task.logger = mock.Mock()
+        task()
+        assert task.return_values["status"] == DataOperationStatus.ROW_FAILURE
+        assert task.return_values["total_row_errors"] == 1
+        assert (
+            task.logger.mock_calls[-1].args[0]
+            == "All account objects processed (2). 1 errors"
+        ), task.logger.mock_calls[-1].args[0]
+
+    @bulkapi_responses.activate
+    def test_update_row_errors_exception_catching(self, create_task):
+        bulkapi_responses.add_query_operation(
+            sobject="Account",
+            fields=["Id", "Name"],
+            query="SELECT Id,Name FROM Account",
+            api=DataApi.SMART,
+            results=[
+                ["OID000BLAH", "Mark Benihoff"],
+                ["OID000BLAH2", "Parkour Hairish"],
+            ],
+        )
+        loader = mock.Mock()
+        bulkapi_responses.add_dml_operation(
+            sobject="Account",
+            operation=DataOperationType.UPDATE,
+            fields=["BillingStreet", "Description", "NumberOfEmployees", "Id"],
+            api_options={},
+            api=DataApi.SMART,
+            volume=2,
+            results=[
+                DataOperationResult("OID000BEEF", success=True, error=""),
+                DataOperationResult("OID000BLAH", success=False, error="humbug"),
+            ],
+            loader_callback=loader,
+            job_result_status=DataOperationStatus.ROW_FAILURE,
+        )
+
+        task = create_task(
+            UpdateData,
+            {
+                "object": "Account",
+                "recipe": "datasets/update.recipe.yml",
+                "fields": ["Name"],
+                "ignore_row_errors": False,
+            },
+        )
+
+        task._validate_and_inject_namespace_prefixes = _fake_val_and_inject_ns
+        with pytest.raises(exc.BulkDataException) as e:
+            task()
+        assert str(e.value) == "1 update error"
+
 
 class TestUpdatesIntegrationTests:
     @pytest.mark.needs_org()