Skip to content

Revert "[meta] do not fail bulk operations if MODE_IGNORE_ERROR (#89)"#96

Merged
prabhataravind merged 1 commit into202506from
paravind/revert_a2b75d8
Oct 30, 2025
Merged

Revert "[meta] do not fail bulk operations if MODE_IGNORE_ERROR (#89)"#96
prabhataravind merged 1 commit into202506from
paravind/revert_a2b75d8

Conversation

@prabhataravind
Copy link
Contributor

This reverts commit 354d29d.

@prabhataravind prabhataravind merged commit da4a0b2 into 202506 Oct 30, 2025
3 checks passed
status = meta_generic_validation_get(meta_key, attr_count[idx], attr_list[idx]);

CHECK_STATUS_SUCCESS_MODE(status, mode);
// FIXME: This macro returns on failure.

Check notice

Code scanning / CodeQL

FIXME comment Note

FIXME comment: This macro returns on failure.

Copilot Autofix

AI 4 months ago

To fix the issue, we need to alter the handling of failed calls to meta_generic_validation_get (and similar places if relevant), so that when the bulk operation error mode is set to SAI_BULK_OP_ERROR_MODE_IGNORE_ERROR, the code DOES NOT return immediately on validation failure. Instead, it should record the error in object_statuses[idx], set the appropriate status (e.g., SAI_STATUS_INVALID_PARAMETER), and continue processing the remaining objects, allowing the bulk operation to complete as expected in the ignore-error mode.

Specifically, replace the CHECK_STATUS_SUCCESS(status); macro (which returns immediately on failure) with explicit handling:

  • If mode is IGNORE_ERROR, record the error in object_statuses[idx] and continue.
  • Otherwise, return the status as before.

Remove the FIXME comment and update the code as appropriate.

Required changes:

  • Remove line 1339's FIXME.
  • Replace lines 1342 (CHECK_STATUS_SUCCESS...) with new logic checking mode.
  • Ensure the handling fits the local code style convention; do not alter lines outside the shown snippet.

Suggested changeset 1
meta/Meta.cpp

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/meta/Meta.cpp b/meta/Meta.cpp
--- a/meta/Meta.cpp
+++ b/meta/Meta.cpp
@@ -1336,11 +1336,22 @@
 
         status = meta_generic_validation_get(meta_key, attr_count[idx], attr_list[idx]);
 
-        // FIXME: This macro returns on failure.
-        // When mode is SAI_BULK_OP_ERROR_MODE_IGNORE_ERROR we should continue instead of return.
-        // This issue exists for all bulk operations.
-        CHECK_STATUS_SUCCESS(status);
-    }
+        if (status != SAI_STATUS_SUCCESS)
+        {
+            if (mode == SAI_BULK_OP_ERROR_MODE_IGNORE_ERROR)
+            {
+                // Record error for this object and continue
+                if (object_statuses != nullptr)
+                {
+                    object_statuses[idx] = status;
+                }
+                continue;
+            }
+            else
+            {
+                return status;
+            }
+        }
 
     auto status = m_implementation->bulkGet(object_type, object_count, object_id, attr_count, attr_list, mode, object_statuses);
 
EOF
@@ -1336,11 +1336,22 @@

status = meta_generic_validation_get(meta_key, attr_count[idx], attr_list[idx]);

// FIXME: This macro returns on failure.
// When mode is SAI_BULK_OP_ERROR_MODE_IGNORE_ERROR we should continue instead of return.
// This issue exists for all bulk operations.
CHECK_STATUS_SUCCESS(status);
}
if (status != SAI_STATUS_SUCCESS)
{
if (mode == SAI_BULK_OP_ERROR_MODE_IGNORE_ERROR)
{
// Record error for this object and continue
if (object_statuses != nullptr)
{
object_statuses[idx] = status;
}
continue;
}
else
{
return status;
}
}

auto status = m_implementation->bulkGet(object_type, object_count, object_id, attr_count, attr_list, mode, object_statuses);

Copilot is powered by AI and may make mistakes. Always verify output.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants