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.

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

Bump psutil and sagemaker-containers version #102

Merged
merged 1 commit into from
Apr 3, 2020
Merged

Conversation

rizwangilani
Copy link
Contributor

@rizwangilani rizwangilani commented Mar 27, 2020

Bump psutil from 5.4.8 to 5.6.7 and sagemaker-containers from 2.5.6 to 2.8.3

Issue #, if available:

Description of changes:

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

requirements.txt Outdated Show resolved Hide resolved
Bump psutil from 5.4.8 to 5.6.7 and sagemaker-containers from 2.5.6 to 2.8.3
@edwardjkim
Copy link
Contributor

edwardjkim commented Mar 27, 2020

  • Did we run integration tests?
  • Does sagemaker-containers 2.8.3 have the fix for the requirements.txt issue? Is there a way for us to test this?
  • Does XGBoost have network isolation?

@aws-patlin
Copy link
Contributor

@edwardjkim Just an FYI, the issues with requirements.txt and network isolation should not affect XGBoost. Those bugs were present in the code path of modules.download_and_install and entrypoint.run, while in XGBoost we call modules.run_module.

@edwardjkim
Copy link
Contributor

Thanks @aws-patlin, that makes sense. Can we just make sure to manually run integration tests as a sanity check before merging since this involves sagemaker-containers?

@rizwangilani rizwangilani merged commit e168eae into master Apr 3, 2020
@rizwangilani rizwangilani deleted the version-bump branch April 3, 2020 16:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants