refractor : enhance workflow robustness and logging

[mansirathee108-cloud]

Fixes N/A
​I refactored the calculate_success method in the BaseWorkflow class.
​I added a logger warning when there are no steps available to calculate success. Previously, the code would return None silently when encountering empty workflows or missing data, making it difficult to debug failures. This change ensures that if a workflow fails to calculate, there is a clear log entry explaining why.
​Type of change:
​[x] Improvement
​Checklist:
​[x] I have read the CONTRIBUTING document.
​[ ] My PR title is Fixes :
​[ ] I have commented on my code, particularly in hard-to-understand areas.
​[ ] For JSON Schema changes: I updated the migration scripts or explained why it is not needed.

---

Summary by Gitar

• Robustness improvements:
  • Added filtering to calculate_success to ignore None values and prevent potential errors in mean calculations.
  • Added a null check when retrieving start_time in print_status to handle cases with empty steps.
• Logging enhancements:
  • Updated error logging in exception handling to include the ingestionPipelineFQN for better traceability.

_{This will update automatically on new commits.}

[github-actions]

Hi there 👋 Thanks for your contribution!

The OpenMetadata team will review the PR shortly! Once it has been labeled as safe to test, the CI workflows
will start executing and we'll be able to make sure everything is working as expected.

Let us know if you need any help!

[gitar-bot]

⚠️ Bug: calculate_success() returning None silently marks workflow as success

When calculate_success() returns None (all steps return None, or no steps), the success is not None guard at line 270 skips the partialSuccess check. The pipeline_state defaults to PipelineState.success (line 263), meaning a workflow where success could not be calculated is reported as fully successful rather than flagging an issue. The old code would have raised a TypeError (comparing None with <=), which would have been caught and set the state to failed. The new behavior is arguably wrong: an incalculable success metric should not default to success.

Suggested fix:

success = self.calculate_success()

if success is None:
    logger.warning("Could not calculate success for the workflow")
elif self.workflow_config.successThreshold <= success < 100:
    pipeline_state = PipelineState.partialSuccess

_{Was this helpful? React with 👍 / 👎 | Reply gitar fix to apply this suggestion}

[gitar-bot]

Code Review ⚠️ Changes requested 0 resolved / 1 findings

Redundant calls to calculate_success() in list comprehension impact efficiency, while a logic flaw allows the workflow to silently default to success when the function returns None.

⚠️ Bug: calculate_success() returning None silently marks workflow as success

📄 ingestion/src/metadata/workflow/base.py:263 📄 ingestion/src/metadata/workflow/base.py:268-270 📄 ingestion/src/metadata/workflow/base.py:215-218

When calculate_success() returns None (all steps return None, or no steps), the success is not None guard at line 270 skips the partialSuccess check. The pipeline_state defaults to PipelineState.success (line 263), meaning a workflow where success could not be calculated is reported as fully successful rather than flagging an issue. The old code would have raised a TypeError (comparing None with <=), which would have been caught and set the state to failed. The new behavior is arguably wrong: an incalculable success metric should not default to success.

Suggested fix

success = self.calculate_success()

if success is None:
    logger.warning("Could not calculate success for the workflow")
elif self.workflow_config.successThreshold <= success < 100:
    pipeline_state = PipelineState.partialSuccess

🤖 Prompt for agents

Code Review: Redundant calls to calculate_success() in list comprehension impact efficiency, while a logic flaw allows the workflow to silently default to success when the function returns None.

1. ⚠️ Bug: calculate_success() returning None silently marks workflow as success
   Files: ingestion/src/metadata/workflow/base.py:263, ingestion/src/metadata/workflow/base.py:268-270, ingestion/src/metadata/workflow/base.py:215-218

   When `calculate_success()` returns `None` (all steps return None, or no steps), the `success is not None` guard at line 270 skips the `partialSuccess` check. The pipeline_state defaults to `PipelineState.success` (line 263), meaning a workflow where success could not be calculated is reported as fully successful rather than flagging an issue. The old code would have raised a `TypeError` (comparing None with <=), which would have been caught and set the state to `failed`. The new behavior is arguably wrong: an incalculable success metric should not default to success.

   Suggested fix:
   success = self.calculate_success()
   
   if success is None:
       logger.warning("Could not calculate success for the workflow")
   elif self.workflow_config.successThreshold <= success < 100:
       pipeline_state = PipelineState.partialSuccess

Options

Display: compact → Showing less information.

Comment with these commands to change:

Compact

gitar display:verbose

_{Was this helpful? React with 👍 / 👎 | Gitar}