Skip to content

Switch to new wheel building pipeline#3731

Merged
rapids-bot[bot] merged 6 commits intorapidsai:branch-23.08from
vyasr:feat/local_ci
Jul 24, 2023
Merged

Switch to new wheel building pipeline#3731
rapids-bot[bot] merged 6 commits intorapidsai:branch-23.08from
vyasr:feat/local_ci

Conversation

@vyasr
Copy link
Contributor

@vyasr vyasr commented Jul 21, 2023

Moves the wheel build and test logic out of the workflow into the repo. This matches conda tests more closely and allows each repo to manage its own wheels more easily.

@vyasr vyasr added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Jul 21, 2023
@vyasr vyasr requested a review from a team as a code owner July 21, 2023 16:46
@vyasr vyasr self-assigned this Jul 21, 2023
@BradReesWork BradReesWork added this to the 23.08 milestone Jul 24, 2023
@BradReesWork
Copy link
Member

/merge

@rapids-bot rapids-bot bot merged commit 78ec476 into rapidsai:branch-23.08 Jul 24, 2023
Copy link
Member

@ajschmidt8 ajschmidt8 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leaving a post-merge review

#!/bin/bash
# Copyright (c) 2023, NVIDIA CORPORATION.

set -eoxu pipefail
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
set -eoxu pipefail
set -euo pipefail

Do we need the -x here? Or can we keep it consistent with the other scripts?

#!/bin/bash
# Copyright (c) 2023, NVIDIA CORPORATION.

set -eoxu pipefail
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
set -eoxu pipefail
set -euo pipefail

Do we need the -x here? Or can we keep it consistent with the other scripts?

#!/bin/bash
# Copyright (c) 2023, NVIDIA CORPORATION.

set -eoxu pipefail
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
set -eoxu pipefail
set -euo pipefail

Do we need the -x here? Or can we keep it consistent with the other scripts?


RAPIDS_PY_CUDA_SUFFIX="$(rapids-wheel-ctk-name-gen ${RAPIDS_CUDA_VERSION})"

bash ci/release/apply_wheel_modifications.sh ${version_override} "-${RAPIDS_PY_CUDA_SUFFIX}"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we remove the bash keyword here?

This will require that the called script has a proper shebang (#!/bin/bash) and is executable (chmod +x <file>).

arch=$(uname -m)
if [[ "${arch}" == "x86_64" ]]; then
pushd ./datasets
bash ./get_test_data.sh
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we remove the bash keyword here?

This will require that the called script has a proper shebang (#!/bin/bash) and is executable (chmod +x <file>).

Comment on lines +15 to +16
arch=$(uname -m)
if [[ "${arch}" == "x86_64" ]]; then
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
arch=$(uname -m)
if [[ "${arch}" == "x86_64" ]]; then
if [[ "$(arch)" == "aarch64" && ${RAPIDS_BUILD_TYPE} == "pull-request" ]]; then

arch and uname -m return the same value, so this can be shortened.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

improvement Improvement / enhancement to an existing function non-breaking Non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants