GEN-1166 - Improve Ingestion Workflow Error Summary #18280
Conversation
github-actions
bot
added
Ingestion
safe to test
| ### Python SDK | ||
| The `metadata insight` command has been removed. Since Data Insights application was moved to be an internal system application instead of relying on external pipelines the SDK command to run the pipeline was removed. | ||
| Now, we're changing this behavior to consider the success rate of all the steps involved in the workflow. The UI will | ||
| then show more `Partial Success` statuses rather than `Failed`, properly reflecting the real state of the workflow. |
There was a problem hiding this comment.
I dont understand how this impacts me as a user. Will I have more failing ingestions? Less? We can take an example ingestion and show how it would reflect differently now compared to before this change.
There was a problem hiding this comment.
mhmhmh it was the goal of this phrasing
The UI will then show more `Partial Success` statuses rather than `Failed`
-> less Failed, more Partial Success.
Basically wanted to give a heads up that if they had a pipeline that before showed as failed and now it showed something different, it was expected 🤔
There was a problem hiding this comment.
approved, as a side quest I suggest we clarify definition of "partial success" or do away with it. From my experience ETL processes have a status of success / failure so the user knows if they need to inverine or not. I have suspicions that "partial success" can confuse platform users of whether they need to do something about their pipelines.
Examples of this design can be seen in:
- Bash - nonzero for failure
- Airflow state machine
- AWS Lambda
- Dagster - a bit less orthodox but a boolean value.
|
|
||
| self._print_failures_if_apply(failures) | ||
|
|
||
| total_success = max(total_records, 1) | ||
| # If nothing is processed, we'll have a success of 100% | ||
| success_pct = mean([step.get_status().calculate_success() for step in steps]) |
There was a problem hiding this comment.
this will fail with this error if steps is empty.
statistics.StatisticsError: mean requires at least one data point
Not sure if we currently have such a use-case but worth handling this just in case and ask the users to check config or send them to slack.
There was a problem hiding this comment.
should not happen since we pass the steps by hand but fair enough, will handle that
|
* GEN-1166 - Improve Ingestion Workflow Error Summary * fix test * docs * comments
* GEN-1166 - Improve Ingestion Workflow Error Summary * fix test * docs * comments
Describe your changes:
Adding success % to the status list
and unifying how we compute success %.
Also, final % is based as the mean of the success of all steps. Before, we only gave "wiggle room" for failures on the source, but not on any other step. Now, the SUCCESS_THRESHOLD is applied to all steps, and final success % is based on all the steps.
Finally, we are allowing to configure the success % in the workflowConfig instead of having it as a constant in the code.
Type of change:
Checklist:
Fixes <issue-number>: <short explanation>