Conversation
Signed-off-by: Kevin Minehart <5140827+kminehart@users.noreply.github.com>
|
@kminehart We also have some problems with the Error 429. In our case we do use dagger-for-github with a specified version, not latest, and after checking the code we could see that an unnecessary curl was done to fetch the latest version. We are going to try out our solution but will look into yours as well. |
|
@kminehart Even after trying the new v8.4.1 we do have issues with error 429. |
|
Yeah, we pin a specific dagger version at the moment. This change did help us quite a lot. |
|
Nice! It looks like we did not pin the dagger version in our workflows, and then the situation is unchanged. But I tested to use this PR and then the caching helped. I should pin the version as well, to get the benefit of my own fix ;-) How does the plan look like for this PR? |
| # Add 'v' prefix if version doesn't start with 'v' and is not empty | ||
| VERSION="v$VERSION" | ||
| fi | ||
| latest=$(curl https://dl.dagger.io/dagger/versions/latest) |
There was a problem hiding this comment.
Needs rebase towards latest main, to include the avoid-curl fix.
| uses: actions/cache/restore@v4 | ||
| with: | ||
| path: ${{ runner.temp }}/dagger-cache | ||
| key: dagger-${{ runner.os }}-${{ inputs.version }}-${{ inputs.commit }} |
There was a problem hiding this comment.
I am wondering what happens if version and commit both are specified?
For key should it be that OS + commit used if commit is specified, or OS + version if commit is not specified?
I could not find any checks that commit matches version.
There was a problem hiding this comment.
Another thing:
- What if version is "latest"
- And commit is not specified
Does this means that we will use a cached latest version? Which will not be updated if latest changes?
|
I had two general comments:
I am interested to approve this commit. |
| curl -fsSL https://dl.dagger.io/dagger/install.sh | \ | ||
| BIN_DIR=${prefix_dir}/bin DAGGER_VERSION="$VERSION" DAGGER_COMMIT="$COMMIT" sh | ||
| if [[ "$CACHE_BINARY" == "true" ]]; then | ||
| mkdir -p "${cache_dir}/bin" "${prefix_dir}/bin" |
There was a problem hiding this comment.
Do we need to ensure these directories does not already exists? We could add a rm before just in case.
2 participants
We're running into 429s on our self-hosted runners from dagger.io. Caching the dagger archive in the github actions cache should help us quite a lot.
I've added the
cache-binaryinput (specifically named so as to not confuse folks with the BuildKit cache) and defaulted it to false for backwards compatibility.