-
Notifications
You must be signed in to change notification settings - Fork 835
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
Update Storage.py and initialiser image #1368
Conversation
Fri Jan 24 17:46:58 UTC 2020 impatient try |
Fri Jan 24 17:47:06 UTC 2020 impatient try |
8e9f47e
to
5205434
Compare
I've rebased from |
Tue Jan 28 13:17:50 UTC 2020 impatient try |
/cc @axsaucedo |
Tue Jan 28 13:18:02 UTC 2020 impatient try |
@adriangonz this looks solid. I think it would be best to add this PR into the KFServing repo, and update the image to use the latest KFServing version of the image downloader - at least that's how we're doing it atm |
/hold |
@axsaucedo this PR just copies over the latest version of KFServing's The only change wrt their repo is adding support for different names of the environment variable which disables SSL in S3 links. But that's mainly so that old versions of Seldon Core don't break with this change. |
/hold cancel |
/approve |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: axsaucedo The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Tue Feb 4 21:53:43 UTC 2020 impatient try |
Tue Feb 4 21:53:51 UTC 2020 impatient try |
failed to trigger Pull Request pipeline
|
Fixes #1341
Changelog
Storage.py
from KFServing, including tests.TODO
kfserving/storage-initializer
image.Notes
To make SSL optional in S3 URLs, both Seldon and KFServing introduced different environment variables with the same finality:
USE_SSL
andS3_USE_HTTPS
. You can find the respective PRs here:In order to make
Storage.py
compatible with thekfserving/storage-initializer
image and to avoid any breaking changes on users who were already using Seldon'sUSE_SSL
, I've made both compatible (with priority forUSE_SSL
).