-
Notifications
You must be signed in to change notification settings - Fork 114
Adding multi arch buildspec for Codebuild setup #132
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
Conversation
baa7052
to
18bf14e
Compare
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's the test/resources/module/user_code.tar.gz
for?
ci/buildspec_arm.yml
Outdated
- docker run --rm -t test-sklearn sh -c 'pytest --cov=sagemaker_sklearn_container --cov-fail-under=60 test/unit' | ||
- docker run --rm -t test-sklearn sh -c 'flake8 setup.py src test' |
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.
This is minor and I know this is carryover from the other buildspecs, but we aren't actually running tox here.
- docker run --rm -t test-sklearn sh -c 'pytest --cov=sagemaker_sklearn_container --cov-fail-under=60 test/unit' | |
- docker run --rm -t test-sklearn sh -c 'flake8 setup.py src test' | |
- docker run --rm -t test-sklearn sh -c 'tox -e ALL' |
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.
test/resources/module/user_code.tar.gz
is used by the integ-tests
ci/buildspec_arm.yml
Outdated
commands: | ||
- echo Build completed on `date` | ||
- echo Pushing the Docker image... | ||
- docker push 515193369038.dkr.ecr.us-west-2.amazonaws.com/sagemaker-scikit-learn:$FRAMEWORK_VERSION-arm-cpu-py3 |
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.
Please change this to "arm64" to be consistent with XGBoost or the other way around. What it actually is (arm, arm64, etc.) does not really matter to me, but I think consistency across images would be good.
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.
made it consistent with XGBoost (arm64)
18bf14e
to
19dfdcb
Compare
Issue #, if available:
To support graviton codebuild, we need graviton specific build specs.
Description of changes: