Skip to content

Commit

Permalink
GH-41481: [CI] Update how extra environment variables are specified f…
Browse files Browse the repository at this point in the history
…or the integration test docker job (#42009)

### Rationale for this change

Currently, nanoarrow and Rust are not being included in the integration tests. The command issued by archery includes multiple environment variable definitions and the rightmost ones disable the extra environment variables.

https://github.com/apache/arrow/actions/runs/9397807525/job/25881776553#step:9:353

```
DEBUG:archery:Executing `['docker', 'run', '--rm', '-e', 'ARCHERY_DEFAULT_BRANCH=main', '-e', 'ARCHERY_INTEGRATION_WITH_NANOARROW=1', '-e', 'ARCHERY_INTEGRATION_WITH_RUST=1', '-e', 'ARCHERY_INTEGRATION_WITH_NANOARROW=0', '-e', 'ARCHERY_INTEGRATION_WITH_RUST=0', '-e', 'ARROW_CPP_EXE_PATH=/build/cpp/debug', '-e', 'ARROW_NANOARROW_PATH=/build/nanoarrow', '-e', 'ARROW_RUST_EXE_PATH=/build/rust/debug', '-e', 'CCACHE_COMPILERCHECK=content', '-e', 'CCACHE_COMPRESS=1', '-e', 'CCACHE_COMPRESSLEVEL=6', '-e', 'CCACHE_DIR=/ccache', '-e', 'CCACHE_MAXSIZE=1G', '-e', 'GITHUB_ACTIONS=true', '-v', '/home/runner/work/arrow/arrow:/arrow', '-v', '/home/runner/work/arrow/arrow/.docker/conda-ccache:/ccache', 'apache/arrow-dev:amd64-conda-integration', '/arrow/ci/scripts/integration_arrow_build.sh /arrow /build && /arrow/ci/scripts/integration_arrow.sh /arrow /build']`
# ...
+ /arrow/ci/scripts/rust_build.sh /arrow /build
=====================================================================
Not building the Rust implementation.
=====================================================================
+ /arrow/ci/scripts/nanoarrow_build.sh /arrow /build
=====================================================================
Not building nanoarrow
=====================================================================
```

### What changes are included in this PR?

This PR updates how environment variables are specified such that the intended value is passed to the docker build.

### Are these changes tested?

Yes

### Are there any user-facing changes?

No
* GitHub Issue: #41481

Lead-authored-by: Dewey Dunnington <[email protected]>
Co-authored-by: Dewey Dunnington <[email protected]>
Co-authored-by: Sutou Kouhei <[email protected]>
Signed-off-by: Sutou Kouhei <[email protected]>
  • Loading branch information
paleolimbot and kou authored Jul 16, 2024
1 parent 6845bb6 commit 21238a7
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 12 deletions.
4 changes: 3 additions & 1 deletion ci/docker/conda-integration.dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,10 @@ RUN mamba install -q -y \

# Install Rust with only the needed components
# (rustfmt is needed for tonic-build to compile the protobuf definitions)
# GH-41637: Version pinned at 1.77 because the glibc for conda-cpp is currently too old
RUN curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- --profile=minimal -y && \
$HOME/.cargo/bin/rustup toolchain install stable && \
$HOME/.cargo/bin/rustup override set 1.77 && \
$HOME/.cargo/bin/rustup toolchain install 1.77 && \
$HOME/.cargo/bin/rustup component add rustfmt

ENV GOROOT=/opt/go \
Expand Down
25 changes: 14 additions & 11 deletions dev/archery/archery/docker/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -340,18 +340,9 @@ def run(self, service_name, command=None, *, env=None, volumes=None,
service = self.config.get(service_name)

args = []
if user is not None:
args.extend(['-u', user])

if env is not None:
for k, v in env.items():
args.extend(['-e', '{}={}'.format(k, v)])

if volumes is not None:
for volume in volumes:
args.extend(['--volume', volume])

if self.config.using_docker or service['need_gpu'] or resource_limit:
use_docker = self.config.using_docker or service['need_gpu'] or resource_limit
if use_docker:
# use gpus, requires docker>=19.03
if service['need_gpu']:
args.extend(['--gpus', 'all'])
Expand Down Expand Up @@ -392,6 +383,18 @@ def run(self, service_name, command=None, *, env=None, volumes=None,
args.append(f'--memory={memory}')
args.append(f'--memory-swap={memory}')

if user is not None:
args.extend(['-u', user])

if env is not None:
for k, v in env.items():
args.extend(['-e', '{}={}'.format(k, v)])

if volumes is not None:
for volume in volumes:
args.extend(['--volume', volume])

if use_docker:
# get the actual docker image name instead of the compose service
# name which we refer as image in general
args.append(service['image'])
Expand Down

0 comments on commit 21238a7

Please sign in to comment.