Conversation
Fixes #6.
There was a problem hiding this comment.
How about test.yml instead of java.yml?
.env
Outdated
| # coredump generation set it to 0 | ||
| ULIMIT_CORE=-1 | ||
|
|
||
| # Default versions for platforms |
.env
Outdated
| ARCH_SHORT=amd64 | ||
|
|
||
| # Default repository to pull and push images from | ||
| REPO=apache/arrow-dev |
There was a problem hiding this comment.
How about using ghcr.io/apache/arrow-java-dev?
If we use Docker Hub, we need to set Docker Hub secrets.
.github/workflows/java.yml
Outdated
| uses: actions/cache@13aacd865c20de90d75de3b17ebe84f7a17d57d2 # v4.0.0 | ||
| with: | ||
| path: .docker | ||
| key: maven-${{ hashFiles('java/**') }} |
There was a problem hiding this comment.
Could you update the hashFiles() argument?
| restore-keys: maven- | ||
| - name: Execute Docker Build | ||
| env: | ||
| DEVELOCITY_ACCESS_KEY: ${{ secrets.GE_ACCESS_TOKEN }} |
There was a problem hiding this comment.
Yes, we'll need to ask Infra to set it up again - this implements build caching (I'll have to figure out the details). It's not strictly required, though
arrow-format/README.rst
Outdated
| This folder contains binary protocol definitions for the Arrow columnar format | ||
| and other parts of the project, like the Flight RPC framework. | ||
|
|
||
| For documentation about the Arrow format, see the `docs/source/format` | ||
| directory. |
There was a problem hiding this comment.
How about mentioning that this is a copy of https://github.com/apache/arrow/tree/main/format ?
We may want to mention the revision of the copy.
Co-authored-by: Sutou Kouhei <kou@cozmixng.org>
| docker compose run \ | ||
| -e CI=true \ | ||
| -e "DEVELOCITY_ACCESS_KEY=$DEVELOCITY_ACCESS_KEY" \ | ||
| ${{ matrix.image }} |
There was a problem hiding this comment.
We need docker compose pull/docker compose push to cache built images but let's work on it as a separated task.
There was a problem hiding this comment.
Sorry. We don't need this. Because we use existing Docker images...
We can remove REPO from .env.
| uses: actions/cache@6849a6489940f00c2f30c0fb92c6274307ccb58a # v4.1.2 | ||
| with: | ||
| path: .docker | ||
| key: maven-${{ matrix.jdk }}-${{ matrix.maven }}-${{ hashFiles('**/docker-compose.yml') }} |
There was a problem hiding this comment.
We may want to update cache by .java changes. Let's work on it as a separated task.
There was a problem hiding this comment.
Hmm, since the docker image is just jdk/maven, shouldn't it not depend on the actual Java sources?
There was a problem hiding this comment.
I think that this is used to cache mvn ... results. So the cache will depend on *.java too.
There was a problem hiding this comment.
Hmm, the build script starts by wiping the build directory, though. (And we have separate build caching support.)
There was a problem hiding this comment.
Oh, I'm not familiar with Maven cache...
This .docker path caches ~/.m2/ directory. Do we need to cache ~/.m2/? If we don't need to cache ~/.m2/, we can remove this cache.
There was a problem hiding this comment.
Ah, this is confusing then. If it caches .m2 then it'll cache downloaded dependencies, which is useful, and that depends on pom.xml (not .java though)
There was a problem hiding this comment.
I'll update this to hash pom.xml in the new PR
2 participants
Fixes #6.