-
Notifications
You must be signed in to change notification settings - Fork 65
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.
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
#12775: Cleanup docker run action #12777
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -57,8 +57,6 @@ jobs: | |
name: ${{ matrix.test-group.name }} ${{ inputs.arch }} ${{ inputs.runner-label }} | ||
env: | ||
LOGURU_LEVEL: INFO | ||
# may not need this | ||
LD_LIBRARY_PATH: ${{ github.workspace }}/build/lib | ||
runs-on: | ||
- ${{ inputs.runner-label }} | ||
- cloud-virtual-machine | ||
|
@@ -72,12 +70,9 @@ jobs: | |
timeout-minutes: ${{ inputs.timeout }} | ||
uses: ./.github/actions/docker-run | ||
with: | ||
docker_username: ${{ github.actor }} | ||
install_wheel: true | ||
docker_password: ${{ secrets.GITHUB_TOKEN }} | ||
docker_image_arch: ${{ inputs.arch }} | ||
run_args: | | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could combine this into just one line so it's pretty: |
||
WHEEL_FILENAME=$(ls -1 *.whl) | ||
pip3 install --user $WHEEL_FILENAME | ||
${{ matrix.test-group.cmd }} | ||
- uses: ./.github/actions/slack-report | ||
if: ${{ failure() }} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -49,8 +49,6 @@ jobs: | |
name: ${{ matrix.test-group.name }} ${{ inputs.arch }} ${{ inputs.runner-label }} | ||
env: | ||
LOGURU_LEVEL: INFO | ||
# may not need this | ||
LD_LIBRARY_PATH: ${{ github.workspace }}/build/lib | ||
runs-on: | ||
- ${{ inputs.runner-label }} | ||
- in-service | ||
|
@@ -64,12 +62,9 @@ jobs: | |
timeout-minutes: ${{ inputs.timeout }} | ||
uses: ./.github/actions/docker-run | ||
with: | ||
docker_username: ${{ github.actor }} | ||
install_wheel: true | ||
docker_password: ${{ secrets.GITHUB_TOKEN }} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I could not move the docker_password parameter to be filled by default since There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. have we tried using Seem strange that a github action can't get access to custom GH actions There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes, I have tried and the secrets are inaccessible in the Github actions - see here There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes this is true - the |
||
docker_image_arch: ${{ inputs.arch }} | ||
run_args: | | ||
WHEEL_FILENAME=$(ls -1 *.whl) | ||
pip3 install --user $WHEEL_FILENAME | ||
source tests/scripts/run_python_model_tests.sh && run_python_model_tests_${{ inputs.arch }} | ||
- uses: ./.github/actions/slack-report | ||
if: ${{ failure() }} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did we mention in the meeting if we wanted to move the artifact downloading into the docker run action?
I do not know if that makes sense but I also do not like the dependency on the wheel presence that is implicit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what artifact?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The wheel artifact. This action relies on its presence but there is no explicit dependency defined in the Github Acton jobs.