Skip to content

Update workflow to support import list overrides - #711

Merged
vish-cs merged 1 commit into
datacommonsorg:masterfrom
vish-cs:override
Aug 5, 2026
Merged

Update workflow to support import list overrides#711
vish-cs merged 1 commit into
datacommonsorg:masterfrom
vish-cs:override

Conversation

@vish-cs

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

Copy link
Copy Markdown
Contributor

Currently, ingestion workflow takes a list of import names as an argument. It then fetches the latest version information from the spanner ImportStatus table. This PR allows passing an optional latest version path as an input argument for easy backfilling. For this, we make the import list argument support a JSON body (importName and latestVersion). This input path eliminates the need for a spanner look up via an override. As part of this, code is cleaned up to merge graphPath and latestVersion params into a single latestVersion parameter.

@codacy-production

codacy-production Bot commented Aug 4, 2026

Copy link
Copy Markdown

Not up to standards ⛔

🔴 Issues 2 minor

Alerts:
⚠ 2 issues (≤ 0 issues of at least minor severity)

Results:
2 new issues

Category Results
CodeStyle 2 minor

View in Codacy

🟢 Metrics 6 complexity

Metric Results
Complexity 6

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 updates the import workflow pipeline to support structured dictionary objects containing optional overrides for latestVersion and graphPath instead of only plain strings. Feedback on these changes highlights critical issues in get_import_info, including a graph path mismatch when only the version is overridden, a lack of backward compatibility for string inputs, and potential NoneType exceptions. Additionally, a bug was identified in parse_import_list where dictionaries missing the importName key are incorrectly stringified and appended as invalid import names.

Comment thread pipeline/workflow/ingestion-helper/clients/spanner.py
Comment thread pipeline/workflow/aggregation-helper/main.py Outdated
Comment thread pipeline/workflow/ingestion-helper/clients/spanner.py
@vish-cs

vish-cs commented Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

/gemini review

@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 updates the ingestion pipeline and helper workflows to support passing import configurations as a list of dictionaries containing 'importName' and optional 'latestVersion' (representing the full GCS path), rather than just a list of strings. This change simplifies the data structure by consolidating 'graphPath' and 'latestVersion' into a single field. The feedback highlights two main improvement opportunities: first, validating input types in the aggregation helper and raising errors for malformed data instead of silently converting them to strings; second, reducing code duplication in the Spanner client by unifying the database query execution and removing unnecessary defensive string-stripping logic.

Comment thread pipeline/workflow/aggregation-helper/main.py Outdated
Comment thread pipeline/workflow/ingestion-helper/clients/spanner.py
@vish-cs
vish-cs force-pushed the override branch 2 times, most recently from d71a506 to 0ac17a1 Compare August 4, 2026 10:29
Comment thread pipeline/workflow/ingestion-helper/routes/imports.py
Comment thread pipeline/workflow/ingestion-helper/clients/spanner.py
@vish-cs
vish-cs force-pushed the override branch 2 times, most recently from 7bcbc07 to 1beabb4 Compare August 5, 2026 10:48
@vish-cs
vish-cs merged commit 6a51131 into datacommonsorg:master Aug 5, 2026
10 of 11 checks passed
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