-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Build Feast Jupyter image and clean up examples #803
Build Feast Jupyter image and clean up examples #803
Conversation
Funny, I've done exactly the same thing today on our fork :) Here's my dockerfile (maximizing layer reuse). It seems to me that there is no need anymore for the startup script to download and install stuff, and downloading release versions in the dev cycle makes config management harder?
|
@algattik neat! I might reuse some of yours. I will still be cloning the repository since the image I am building is not meant for development, it's meant to have the latest stable dependencies preinstalled, and I guess we can have the actual SDK also preinstalled. I don't like mapping in the examples directly from the repo because master is normally not consistent with the stable branch, and if we pin versions then its very hard to backport tutorials or make small tweaks. |
@algattik Made some changes to find a better middle ground. Hope it looks better :) I didn't attribute the Dockerfile to you, but normally people don't care 🎖️ |
/test test-end-to-end-batch |
command: ["/etc/startup.sh"] | ||
command: > | ||
bash -c " | ||
git clone https://github.com/feast-dev/feast.git || true |
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.
Is there a reason why can't we bake in the Feast repo into the docker image?
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.
The latest
image will only be built on a release, which means that if we want to roll out the latest tutorials then we have to cut a release. Often I find myself updating docs/tutorials between releases, which is why I have pinned this clone
to the branch instead. In theory we could build the latest
image on every commit, but that image will go stale very quickly on the user's side (they have to repull the whole time).
/test test-end-to-end-batch |
infra/docker/jupyter/Dockerfile
Outdated
WORKDIR /feast | ||
|
||
# .git (required by SDK setup.py) | ||
COPY .git .git |
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.
.git and README.md are only needed by " pip install -e sdk/python" IIRC, so if you move them down you can reuse layer with requirements installed. (.git folder changes with every commit)
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.
Corrected, thanks.
My ego shall survive :) |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: woop, zhilingc 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 |
@woop: Updated the
In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
What this PR does / why we need it:
gcr.io/kf-feast/feast-jupyter:latest
) so that it's faster for new users to start the Jupyter container for Docker Compose. Feast dependencies are still installed upon startup, but most of them should now already be available.entity_df
bug in Telco example and added more detailsFixes: #742