-
Notifications
You must be signed in to change notification settings - Fork 43
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
feat(api): Refactor API server to introduce support for Docker image registries and s3 bucket support #605
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #605 +/- ##
==========================================
- Coverage 60.68% 59.71% -0.98%
==========================================
Files 277 277
Lines 22245 26043 +3798
==========================================
+ Hits 13500 15551 +2051
- Misses 7877 9625 +1748
+ Partials 868 867 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
c383ac1
to
3710fc2
Compare
018e395
to
ed37113
Compare
e72009c
to
a63f15f
Compare
a63f15f
to
ae0f929
Compare
4c9ec3f
to
29a1042
Compare
Thanks a lot for the comments @tiopramayudi and @bthari ! I should've covered and addressed all of the outstanding comments. If all looks good feel free to approve it again otherwise let me know what additional changes we should include and I'll get back to it. Thanks once again! |
…configured as an artifact strore
… and fix incorrect if-else logic
c15b98d
to
fe5de0d
Compare
6acd1b4
to
76ecdc5
Compare
Thanks for the re-review @bthari! I'll keep this PR open a little while longer if Tio wants to look at the part I added to the Merlin SDK to allow users to download the Merlin model files from an s3-configured Mlflow blob store. Otherwise I'll merge this by tomorrow! :D |
Note 🚨
This PR should not be merged without the changes in caraml-dev/mlp#116 being merged, published and imported as dependencies first. This PR is currently only using a branch of a fork (the source of the quoted PR) of the MLP repository.The dependent PR has been merged.Description
In order to provide support for using image registries that use Docker registry credentials as well as AWS S3-based blob storage services, this PR refactors the API server to support these additional image registry and bob storage options. More concretely, these are the following changes made:
Modifications
api/cmd/api/setup.go
- Make the initialisation of the image builder set up the artifact service type and docker registry correctly depending on the one set upapi/config/config.go
- Introduce new configs for platform maintainers to specify theKanikoPushRegistryType
and theKanikoDockerCredentialSecretName
api/pkg/imagebuilder/imagebuilder.go
- Make changes to the image builder to configure the Kaniko job spec correctly depending on the selected registry type and blob storage typepython/batch-predictor/docker/app.Dockerfile
- Add steps to the batch predictor docker image to authenticate and pull model artifacts correctly depending on the configured blob storage typepython/batch-predictor/docker/base.Dockerfile
- Add steps to the base batch predictor image to install the AWS CLIpython/pyfunc-server/docker/Dockerfile
- Add steps to the pyfunc server docker image to authenticate and pull model artifacts correctly depending on the configured blob storage typepython/pyfunc-server/docker/base.Dockerfile
- Add steps to the base pyfunc server image to install the AWS CLITests
Checklist
Release Notes