Skip to content

Updating CI Actions path - #276

Merged
jbonofre merged 1 commit into
apache:mainfrom
sfc-gh-swalia:swalia-ci-path-filter-fix
Jul 28, 2026
Merged

Updating CI Actions path#276
jbonofre merged 1 commit into
apache:mainfrom
sfc-gh-swalia:swalia-ci-path-filter-fix

Conversation

@sfc-gh-swalia

Copy link
Copy Markdown
Contributor

Summary

While working on PR #259, I noticed that all 8 converter CI workflows use .github/** as a path trigger. This means any PR that touches anything under .github/ triggers every converter's test suite unnecessarily.

This wastes CI resources and creates confusing failures on unrelated PRs (e.g., #259 showed a failing Omni build that had nothing to do with the docs changes).

This fix narrows each workflow's path filter to only reference its own workflow file, so a converter's CI only runs when its converter code or its own workflow definition changes.

Related Issues

#259

@sfc-gh-swalia

Copy link
Copy Markdown
Contributor Author

@khush-bhatia @jbonofre adding this PR since I noticed CI failing for #259. LMK what you think.

@MonkeyCanCode

Copy link
Copy Markdown
Contributor

@khush-bhatia @jbonofre adding this PR since I noticed CI failing for #259. LMK what you think.

Hello @sfc-gh-swalia,

Yes, this was implicitly added by me. The original idea is if there are composite actions (e.g. license , we should re-test all. I am fine with keep it restrict as one failure will now trigger all PR to halt.

Thanks,
Yong Zheng

@jbonofre

jbonofre commented Jul 28, 2026

Copy link
Copy Markdown
Member

The Omni CI failure here isn't from this PR. This PR only narrows the CI path filters, but editing converter-omni-ci.yml triggers the Omni CI, and that surfaces a pre-existing bug on main.

What happens: #113 added a datatype field to Field/Metric and to examples/tpcds_semantic_model.yaml, but it never touched converters/omni, so the Omni converter doesn't know about datatype. On the OSI -> Omni -> OSI roundtrip the field gets dropped, and the result no longer matches the original. That breaks test_osi_roundtrip_up_to_documented_normalizations and test_tpcds_export_matches_expected.

It slipped in because #113 changed neither converters/omni nor .github, so Omni CI never ran for it.

The path change in this PR is fine. We just need a separate fix to make the Omni converter carry datatype through the conversion (and refresh the tpcds_omni fixture) to get this green.

@jbonofre
jbonofre merged commit 9a7cbef into apache:main Jul 28, 2026
21 of 26 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.

3 participants