Enable AWSSDK on Linux by statically linking OpenSSL and cURL #421
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
facebook-github-bot
added
the
CLA Signed
ejguan
force-pushed
the
py_manylinux
branch
24 times, most recently
from
ef8968f to
0453f12
Compare
ejguan
changed the title
ejguan
force-pushed
the
py_manylinux
branch
3 times, most recently
from
ae88807 to
21abff4
Compare
ejguan
changed the title
|
@ejguan has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
ejguan
requested review from
ydaiming,
VitalyFedyunin,
msaroufim,
NivekT and
bearzx
NivekT
approved these changes
May 26, 2022
| @@ -65,7 +62,7 @@ jobs: | |||
| run: | | |||
| python setup.py install | |||
| env: | |||
| BUILD_S3: ${{ matrix.with-s3 }} | |||
| BUILD_S3: 1 | |||
There was a problem hiding this comment.
Is there any situation where we do not enable BUILD_S3?
It looks like only when the setup is Windows with conda, why is that?
There was a problem hiding this comment.
For binary releases, it's just because there are too many Error that Idk how to solve with AWS enabled for conda on windows.
For CI testing, you can treat it as wheels, we could enable testing with awssdk.
Generally, the reason I think it makes sense to always enable AWSSDK in our testing CI is the >95% tests are the same with or without AWSSDK enabled.
By simply removing one test case of |
ejguan
added a commit
to ejguan/data
that referenced
this pull request
May 31, 2022
…h#421) Summary: This PR adds all supports to enable awssdk for linux releases - The linux release workflow starts to use `pytorch/manylinux-cpu` docker to align with compiler version with PyTorch Core - Add shell script to install OpenSSL and cURL on docker image (This step can be improved if TorchData wants to own a new docker image with these two libraries pre-built) - Update `torchdata/csrc/CMakeLists.txt` to respect the static OpenSSL and cURL - Correct `thrid_party/CMakeLists.txt` without adding redundant dependencies for `_torchdata` - Add `auditwheel show` to validate the binaries are `manylinux_2_17` (alias of `manylinux2014`) - Update Readme for release - Make our testing CI running with AWSSDK enabled by default to save more time/money to run CI tests - Remove redundant `numpy` dependency introduced by `tf_record`. And, fix tests in this file See the workflow for night release: https://github.com/pytorch/data/actions/runs/2391843245 See the `manylinux` version: https://github.com/pytorch/data/runs/6612861049?check_suite_focus=true#step:9:67 `manylinux_2_17_x86_64` is the alias of `manylinux_2014` Pull Request resolved: pytorch#421 Reviewed By: NivekT Differential Revision: D36641092 Pulled By: ejguan fbshipit-source-id: 349bfd896ee0db01eea849580984f4000ca2bc3f
ejguan
added a commit
that referenced
this pull request
May 31, 2022
Summary: This PR adds all supports to enable awssdk for linux releases - The linux release workflow starts to use `pytorch/manylinux-cpu` docker to align with compiler version with PyTorch Core - Add shell script to install OpenSSL and cURL on docker image (This step can be improved if TorchData wants to own a new docker image with these two libraries pre-built) - Update `torchdata/csrc/CMakeLists.txt` to respect the static OpenSSL and cURL - Correct `thrid_party/CMakeLists.txt` without adding redundant dependencies for `_torchdata` - Add `auditwheel show` to validate the binaries are `manylinux_2_17` (alias of `manylinux2014`) - Update Readme for release - Make our testing CI running with AWSSDK enabled by default to save more time/money to run CI tests - Remove redundant `numpy` dependency introduced by `tf_record`. And, fix tests in this file See the workflow for night release: https://github.com/pytorch/data/actions/runs/2391843245 See the `manylinux` version: https://github.com/pytorch/data/runs/6612861049?check_suite_focus=true#step:9:67 `manylinux_2_17_x86_64` is the alias of `manylinux_2014` Pull Request resolved: #421 Reviewed By: NivekT Differential Revision: D36641092 Pulled By: ejguan fbshipit-source-id: 349bfd896ee0db01eea849580984f4000ca2bc3f
bushshrub
pushed a commit
to bushshrub/data
that referenced
this pull request
Jun 2, 2022
…h#421) Summary: This PR adds all supports to enable awssdk for linux releases - The linux release workflow starts to use `pytorch/manylinux-cpu` docker to align with compiler version with PyTorch Core - Add shell script to install OpenSSL and cURL on docker image (This step can be improved if TorchData wants to own a new docker image with these two libraries pre-built) - Update `torchdata/csrc/CMakeLists.txt` to respect the static OpenSSL and cURL - Correct `thrid_party/CMakeLists.txt` without adding redundant dependencies for `_torchdata` - Add `auditwheel show` to validate the binaries are `manylinux_2_17` (alias of `manylinux2014`) - Update Readme for release - Make our testing CI running with AWSSDK enabled by default to save more time/money to run CI tests - Remove redundant `numpy` dependency introduced by `tf_record`. And, fix tests in this file See the workflow for night release: https://github.com/pytorch/data/actions/runs/2391843245 See the `manylinux` version: https://github.com/pytorch/data/runs/6612861049?check_suite_focus=true#step:9:67 `manylinux_2_17_x86_64` is the alias of `manylinux_2014` Pull Request resolved: pytorch#421 Reviewed By: NivekT Differential Revision: D36641092 Pulled By: ejguan fbshipit-source-id: 349bfd896ee0db01eea849580984f4000ca2bc3f
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds all supports to enable awssdk for linux releases
pytorch/manylinux-cpudocker to align with compiler version with PyTorch Coretorchdata/csrc/CMakeLists.txtto respect the static OpenSSL and cURLthrid_party/CMakeLists.txtwithout adding redundant dependencies for_torchdataauditwheel showto validate the binaries aremanylinux_2_17(alias ofmanylinux2014)See the successful workflow for night release: https://github.com/pytorch/data/actions/runs/2391843245
See the
manylinuxversion: https://github.com/pytorch/data/runs/6612861049?check_suite_focus=true#step:9:67manylinux_2_17_x86_64is the alias ofmanylinux_2014