chore(ci): run credentialed Snowflake tests daily - #1014
Conversation
farazdagi
commented
Sep 7, 2026
- Keep credential-free Snowflake tests on relevant PR and main changes, preserving Snowflake Gate.
- Remove Snowflake credentials from routine CI.
- Run credentialed integration tests daily on the default branch, with a 60-minute timeout.
There was a problem hiding this comment.
Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.
Beyond the inline README finding, I also checked the fork-origin if condition removed from snowflake_tests in snowflake-ci.yml — that job no longer receives TESTS_SNOWFLAKE_CONNECTION (it now only runs cargo x test-snowflake --credentials skip), so dropping the fork-origin restriction doesn't expose secrets to fork PRs.
Extended reasoning...
This is the narrow findings-present ruled-out case. The confirmed inline finding (README.md line 67 still attributing TESTS_SNOWFLAKE_CONNECTION to snowflake-ci.yml after the diff moved that secret to snowflake-daily.yml) is real and already queued as an inline comment, so no top-level restatement is warranted. I independently re-verified the workflow diffs: snowflake-ci.yml's snowflake_tests job env no longer sets TESTS_SNOWFLAKE_CONNECTION and the step now runs --credentials skip, while snowflake-daily.yml is schedule-only and carries the credential and its own "Validate Snowflake Secrets" step. Given that, removing the fork-origin if guard on the CI job is safe since no secret is exposed on that path, which addresses one of the two ruled-out candidates supplied to me. I'm surfacing this as a defer note per the narrow exception for concerns examined beyond the reported findings.
Additional findings (outside the current diff — GitHub can't attach inline comments there):
-
🟡
crates/etl-destinations/src/snowflake/README.md— The "CI Configuration" section still says TESTS_SNOWFLAKE_CONNECTION is used for snowflake-ci.yml, but the diff moved that secret to the new snowflake-daily.yml; snowflake-ci.yml no longer reads it. Readers following this doc will wire the secret into the wrong workflow scope, contradicting the new, correct paragraph added earlier in the same file (line 20). Fix: update line 67 to reference .github/workflows/snowflake-daily.yml instead of snowflake-ci.yml.Extended reasoning...
Lines 45-71 of README.md document secret wiring for CI. The diff (line 20 addition) correctly states the daily workflow now owns TESTS_SNOWFLAKE_CONNECTION and routine CI does not receive it, but the pre-existing 'CI Configuration' bullet list at line 67 was not updated and still attributes the secret to snowflake-ci.yml. This directly contradicts the new text a few lines above and matches neither workflow file after the change (snowflake-ci.yml no longer has this env var; snowflake-daily.yml does). A maintainer auditing repo secrets or onboarding a new workflow based on this doc would misconfigure secret scoping.
Verification: nit. Line 67 of README.md still reads "-
TESTS_SNOWFLAKE_CONNECTIONfor.github/workflows/snowflake-ci.yml." but the diff to snowflake-ci.yml removedTESTS_SNOWFLAKE_CONNECTION: ${{ secrets.TESTS_SNOWFLAKE_CONNECTION }}, removed the "Validate Snowflake Secrets" step, and changed the run tocargo x test-snowflake --credentials skip. The secret env var now appears only in the new…