Skip to content
This repository has been archived by the owner on Nov 17, 2023. It is now read-only.

Fix cd by adding to $PATH #19939

Merged
merged 6 commits into from
Feb 24, 2021
Merged

Fix cd by adding to $PATH #19939

merged 6 commits into from
Feb 24, 2021

Conversation

Zha0q1
Copy link
Contributor

@Zha0q1 Zha0q1 commented Feb 20, 2021

#19917
This pr tries to fix ^ this issue. Joe helped update g4 instance to use the new nvidia-460 driver, which should be higher than the minimum requirement of the cu112 docker

@Zha0q1 Zha0q1 requested a review from szha as a code owner February 20, 2021 01:45
@lanking520 lanking520 added pr-awaiting-testing PR is reviewed and waiting CI build and test pr-work-in-progress PR is still work in progress pr-awaiting-review PR is waiting for code review and removed pr-awaiting-testing PR is reviewed and waiting CI build and test pr-work-in-progress PR is still work in progress labels Feb 20, 2021
@lanking520 lanking520 added pr-awaiting-testing PR is reviewed and waiting CI build and test pr-work-in-progress PR is still work in progress and removed pr-awaiting-review PR is waiting for code review pr-awaiting-testing PR is reviewed and waiting CI build and test labels Feb 22, 2021
@Zha0q1 Zha0q1 reopened this Feb 23, 2021
@lanking520 lanking520 added pr-awaiting-testing PR is reviewed and waiting CI build and test and removed pr-work-in-progress PR is still work in progress labels Feb 23, 2021
@Zha0q1 Zha0q1 changed the title [wip] Attemp to fix cd by changing gpu node to g4 which has newer nvidia drivers Fix cd by adding to $PATH Feb 23, 2021
@lanking520 lanking520 added pr-work-in-progress PR is still work in progress pr-awaiting-testing PR is reviewed and waiting CI build and test pr-awaiting-review PR is waiting for code review and removed pr-awaiting-testing PR is reviewed and waiting CI build and test pr-work-in-progress PR is still work in progress labels Feb 23, 2021
filepath=$(readlink -f wheel_build/dist/*.whl)
filename=$(basename $filepath)
variant=$(echo $filename | cut -d'-' -f1 | cut -d'_' -f2 -s)
if [ -z "${variant}" ]; then
variant="cpu"
fi
export PATH=/usr/local/bin:$PATH
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you mean that /usr/local/bin wasn't on the PATH before? Do you know why it worked at all before?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We recently changed the ami used by restricted-mxnetlinux-gpu to update the nvidia driver

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you think if there is a better place to set PATH?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

It's fine to add it here, I'm just trying to understand the reasoning. Can you explain how it's related to AMI? Wouldn't cd_s3_publish run in a docker container and be independent from the AMI?

@Zha0q1 Zha0q1 merged commit 5070f1a into apache:master Feb 24, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
pr-awaiting-review PR is waiting for code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants