Conversation
slinkydeveloper
force-pushed
the
issues/459
branch
4 times, most recently
from
277f843 to
2e2ce6e
Compare
slinkydeveloper
force-pushed
the
issues/459
branch
3 times, most recently
from
5ca7bff to
42ff567
Compare
… that the runner will be different, so we won't have access to the locally built image.
slinkydeveloper
force-pushed
the
issues/459
branch
from
42ff567 to
e43b092
Compare
|
It works :) takes 40 minutes to run though :( But i guess we can iterate on it. |
|
Subsequent run took only 18 minutes though! |
| uses: actions/cache@v3 | ||
| with: | ||
| path: /tmp/.buildx-cache | ||
| key: ${{ runner.os }}-buildx-${{ github.sha }} |
There was a problem hiding this comment.
Perhaps I can replace this with github.head_ref to reuse the cache throughout the same PR? WDYT @tillrohrmann?
There was a problem hiding this comment.
Uh wait perhaps it's not needed?
There was a problem hiding this comment.
Not sure whether I fully understand what your suggestion would change. The key is the id of the cache entry. What is being used by this job is defined via the restore-keys. With this key, we could overwrite the cache entry from the docker.yml workflow since it uses the same key. That might be an argument for using a distinct key.
tillrohrmann
left a comment
tillrohrmann
left a comment
There was a problem hiding this comment.
Thanks for creating this PR @slinkydeveloper. The changes look good to me. I only had a few minor comments. +1 for merging given that you've tried it out and it did the job.
| uses: actions/cache@v3 | ||
| with: | ||
| path: /tmp/.buildx-cache | ||
| key: ${{ runner.os }}-buildx-${{ github.sha }} |
There was a problem hiding this comment.
Not sure whether I fully understand what your suggestion would change. The key is the id of the cache entry. What is being used by this job is defined via the restore-keys. With this key, we could overwrite the cache entry from the docker.yml workflow since it uses the same key. That might be an argument for using a distinct key.
| with: | ||
| images: localhost/restatedev/restate | ||
|
|
||
| - name: Build and push Docker image |
There was a problem hiding this comment.
We are not pushing the docker image, right?
There was a problem hiding this comment.
push: false a couple of lines below
| RESTATE_RUNTIME_CONTAINER: ${{ steps.build.outputs.imageID }} | ||
| NODE_AUTH_TOKEN: ${{ secrets.GH_PACKAGE_READ_ACCESS_TOKEN }} | ||
| with: | ||
| arguments: -Djib.console=plain build |
There was a problem hiding this comment.
would check enough to run all tests?
| # Upload container logs | ||
| - uses: actions/upload-artifact@v3 | ||
| if: always() # Make sure this is run even when test fails | ||
| with: | ||
| name: e2e-container-logs | ||
| path: | | ||
| tests/build/test-results/*/container-logs/** | ||
| tests/build/reports/tests/** | ||
| tests/build/test-results/*/*.xml |
There was a problem hiding this comment.
I think this action could not find the artifacts. At least in the run of the PR there are no uploads. Could it be that the directory e2e is missing in the paths?
There was a problem hiding this comment.
|
I suggest we pause this one for now while we scope out if its possible to call e2e via a cross repo workflow call, so we don't have to duplicate any e2e workflow logic into other repos |
|
Close as superseded by #481 |
3 participants
Fix #459