Add docker-compose file with Jupyer notebook#328
Conversation
|
Hi @khorshuheng. Thanks for your PR. I'm waiting for a gojek member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions 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. |
|
This is awesome, thanks @khorshuheng. Did you run any tests to confirm that this was working? |
77dbbbe to
b92602c
Compare
The run output is included in the quick start jupyter notebook: https://github.com/gojek/feast/blob/b92602c2af2e5599e20e2a5f7bc72e657974b8e4/infra/docker-compose/jupyter/notebooks/feast-quickstart.ipynb I don't think i can write any unit test / integration test for this, but i can potentially include more scenarios in the quick start notebook to test different cases. What other scenarios do you think will be useful for the notebook example? We can also add additional command in the Makefile for starting / stopping the docker compose services, though i am not sure if that is needed for now. |
Hi @khorshuheng. Yea I dont think its necessary to automate the process. We just need to have a basic script where we can apply, ingest, and retrieve successfully. I just want a way to make sure that the docker compose file and configuration works. |
|
/ok-to-test |
e18d4d7 to
23584c0
Compare
There was a problem hiding this comment.
Is there a reason why you are setting allow-dirty to be true?
There was a problem hiding this comment.
My intention is to run the e2e test as follows:
docker-compose -f docker-compose.yml -f docker-compose.override.yml -f docker-compose.e2e.yml run e2e
That way, the e2e test run is entirely self contained. However, the above command will keep the services running even after the e2e test run finished. So for convenience i simply set allow dirty equals to true, otherwise we will need to restart the other services everytime. But i do agree that i should have probably set it to false, for consistency and accuracy of test.
That being say, this e2e test setup will be invalidated once we added test for batch retrieval, because my docker-compose setup has not included batch serving yet, only online serving.
There was a problem hiding this comment.
The only reason I added that flag was for safety, just in case somebody accidentally runs the test while connected to a production cluster. If you think that situation won't occur, then I guess its fine to make it true.
There was a problem hiding this comment.
Is this just running the normal e2e tests?
There was a problem hiding this comment.
Yes. This is also a way to test the docker compose setup.
There was a problem hiding this comment.
Should we perhaps default to latest and just ensure that we always push the latest tagged docker image?
There was a problem hiding this comment.
Not finding this. Did you end up pushing it, or should I help with that?
There was a problem hiding this comment.
I haven't pushed it yet. The current dockerfile always build from master branch, which is fine, but for production we should probably install via pip instead. For which versions should the Feast jupyter images be available? All versions after and including 0.3? (0.3.0, 0.3.2)
Will add reference link to the Gitbook in the README.md |
23584c0 to
c2389c3
Compare
|
Add capability for batch serving using Big Query. Other changes:
|
|
/test test-golang-sdk |
1 similar comment
|
/test test-golang-sdk |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: khorshuheng, woop The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
7411765 to
c613115
Compare
|
/lgtm |
This addresses #272.
This PR added a new Jupyter notebook docker image preinstalled with Feast Python SDK. A quick start notebook has also been included for the corresponding docker compose service.
By default, the docker compose file will not rebuild the image. Instead, it will pull images based on the environmental variable defined under infra/docker-compose/.env. Users can either use the default setting, in which the official Feast images are used, or substitute one or more images with their own local image.
Additional settings, such as google credential key, Springboot configurations can be configured via the .env file as well.
Developers who wish to rebuild everything from scratch may run docker-compose -f docker-compose.yml -f docker-compose.dev.yml up -d.
Pending tasks before this can be merged: