Skip to content

Add rollback to ingestion workflow - #715

Open
vish-cs wants to merge 1 commit into
datacommonsorg:masterfrom
vish-cs:pipeline
Open

Add rollback to ingestion workflow#715
vish-cs wants to merge 1 commit into
datacommonsorg:masterfrom
vish-cs:pipeline

Conversation

@vish-cs

@vish-cs vish-cs commented Aug 6, 2026

Copy link
Copy Markdown
Contributor
  • Update spanner ingestion workflow to add a rollback sub-workflow to handle failures
  • Remove runIngestion parameter to always invoke spanner ingestion workflow from the import automation workflow (batch job)
  • Refactoring/clean up of the workflow

@codacy-production

Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 0 complexity

Metric Results
Complexity 0

View in Codacy

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the import automation and Spanner ingestion workflows to dynamically resolve import payloads, simplify ingestion triggering, and introduce structured sub-workflows for state updates, pipeline execution, and rollback on failure. Additionally, SQL query optimization in the provenance summary generator reduces the IN filter threshold to 20KB with a fallback to stream all node names. Feedback is provided regarding a potential runtime crash in the Spanner ingestion workflow when attempting to retrieve the job ID from a non-map error object during a pipeline failure, along with a suggested try/except block to safely resolve it.

spanner_instance_id: ${spanner_instance_id}
spanner_database_id: ${spanner_database_id}
wait_period: ${wait_period}
job_id: ${default(map.get(pipeline_err, "job_id"), "N/A")}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Using map.get(pipeline_err, "job_id") can cause a runtime crash if pipeline_err is a string (e.g., if an HTTP request fails or a system error occurs) rather than a map. In Google Cloud Workflows, calling map.get on a non-map type throws a TypeError, which would crash the except block itself, preventing the lock from being released and leaving the workflow in an inconsistent state.

To make this robust, we can safely determine the job_id using a try/except step block before calling rollback_subworkflow.

            - determine_job_id:
                try:
                  assign:
                    - failed_job_id: ${pipeline_err.job_id}
                except:
                  as: err
                  assign:
                    - failed_job_id: "N/A"
            - run_rollback:
                call: rollback_subworkflow
                args:
                  workflow_id: '${workflow_id}'
                  helper_url: ${helper_url}
                  import_list: ${import_info_list}
                  project_id: ${project_id}
                  dataflow_job_name: ${dataflow_job_name}
                  dataflow_gcs_path: ${dataflow_gcs_path}
                  location: ${location}
                  spanner_project_id: ${spanner_project_id}
                  spanner_instance_id: ${spanner_instance_id}
                  spanner_database_id: ${spanner_database_id}
                  wait_period: ${wait_period}
                  job_id: ${failed_job_id}
                  stages: ${stages}

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.

1 participant