Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

GH-41481: [CI] Update how extra environment variables are specified for the integration test docker job #42009

Merged
merged 10 commits into from
Jul 16, 2024
Merged

GH-41481: [CI] Update how extra environment variables are specified for the integration test docker job #42009

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
f5157cf
try specifying via env and not via command line
paleolimbot Jun 6, 2024
365a4a6
one more approach
paleolimbot Jun 6, 2024
e9c0994
ensure int expression
paleolimbot Jun 6, 2024
530d5ce
maybe fix rust version
paleolimbot Jun 6, 2024
82f6eea
maybe use the old one by default
paleolimbot Jun 6, 2024
8b2f972
try specifying
paleolimbot Jun 6, 2024
d71259c
Apply suggestions from code review
paleolimbot Jun 11, 2024
e4b1fa1
revert changes
paleolimbot Jun 17, 2024
1c1b7c1
apply kou's suggestion
paleolimbot Jun 17, 2024
74b2f96
try kou's suggestion
paleolimbot Jun 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion ci/docker/conda-integration.dockerfile
View file
Open in desktop
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
View file
Open in desktop
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
Loading