Skip to content

Conversation

carlopi
Copy link
Contributor

@carlopi carlopi commented Sep 25, 2025

Current situation:

       save: ${{ github.ref == 'refs/heads/main' || github.repository != 'duckdb/duckdb' }}

that translate to either being in the main branch OR being in a repository different than duckdb/duckdb.

This aims at achieving a few goals:

  1. avoid PRs to unnecessarily populate the cache, with the risk of evicting more valuable resources from base branches. It assume only main is a base branch Note: PR can use caches from base branches, but not the other way around, so base branches have more value
  2. enforces the rule only for duckdb/duckdb, with the idea that forks are likely low traffice

Proposed change:

       save: ${{ vars.BRANCHES_TO_BE_CACHED == '' || contains(vars.BRANCHES_TO_BE_CACHED, github.ref) }}

combined with defining, at the GitHub duckdb/duckdb repo or at the duckdb org level a variable (not a secret) shaped like:

       BRANCHES_TO_BE_CACHED = ["refs/heads/main", "refs/heads/v1.4-andium"]

This keeps 1, but improves from only main to either one of the currently active branches. This allows also more flexibility on introducing new branches (just a matter of adding one) or in moving branches out of support.

This improves also on 2, given the same behaviour stays, but whoever forked DuckDB can control what wants / needs cached.

I have tested this change in my repository, both without a variable or with a different combination of branches, and seems to behave as intended

…variable that dictate what branches should be cached

Current situation:
```
       save: ${{ github.ref == 'refs/heads/main' || github.repository != 'duckdb/duckdb' }}
```
that translate to either being in the `main` branch OR being in a repository different than duckdb/duckdb.

This aims at achieving a few goals:
1. avoid PRs to unnecessarily populate the cache, with the risk of evicting more valuable resources from base branches. It assume only `main` is a base branch
	Note: PR can use caches from base branches, but not the other way around, so base branches have more value
2. enforces the rule only for duckdb/duckdb, with the idea that forks are likely low traffice

Proposed change:
```
       save: ${{ vars.BRANCHES_TO_BE_CACHED == '' || contains(vars.BRANCHES_TO_BE_CACHED, github.ref) }}
```
combined with defining, at the GitHub duckdb/duckdb repo or at the duckdb org level a variable (not a secret) shaped like:
```
       BRANCHES_TO_BE_CACHED = ["refs/heads/main", "refs/heads/v1.4-andium"]
```

This keeps 1, but improves from only `main` to either one of the currently active branches.
This allows also more flexibility on introducing new branches (just a matter of adding one) or in moving branches out of support.

This improves also on 2, given the same behaviour stays, but whoever forked DuckDB can control what wants / needs cached.

I have tested this change in my repository, both without a variable or with a different combination of branches, and seems to behave as intended
@duckdb-draftbot duckdb-draftbot marked this pull request as draft September 25, 2025 21:41
@carlopi carlopi marked this pull request as ready for review September 25, 2025 21:41
@carlopi carlopi changed the title Change logic for saving caches to consulting an optional REPO or ORG … Change logic for saving caches: Github variable that decides what gets cached Sep 25, 2025
@carlopi carlopi changed the title Change logic for saving caches: Github variable that decides what gets cached [ci] Change logic for saving caches: Github variable that decides what gets cached Sep 25, 2025
@Mytherin Mytherin merged commit d168b57 into duckdb:v1.4-andium Sep 26, 2025
126 of 133 checks passed
@Mytherin
Copy link
Collaborator

Thanks!

krlmlr added a commit to duckdb/duckdb-r that referenced this pull request Sep 29, 2025
Fix handling of quotes in ToString() of search_path in current_setting (duckdb/duckdb#19162)
[ci] Change logic for saving caches: Github variable that decides what gets cached (duckdb/duckdb#19150)
Allow implicit casts from `JSON[]` to `JSON` again (duckdb/duckdb#19141)
Fix bug in merge into when condition is in parenthesis (duckdb/duckdb#19137)
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