Deduplicate example notebooks #456
Conversation
woop
requested review from
davidheryanto,
khorshuheng,
pradithya and
zhilingc
as code owners
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: woop 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 |
|
https://docs.feast.dev/getting-started/installing-feast needs to be updated to direct the user to use the basic notebook instead of quickstart. |
| @@ -4,9 +4,9 @@ services: | |||
| batch-serving: | |||
| image: ${FEAST_SERVING_IMAGE}:${FEAST_VERSION} | |||
There was a problem hiding this comment.
Previously, i separated the docker compose file into two because i thought there will be use cases where users would only want to start online serving and not batch serving. Now that i am more familiar with Feast i think that doesn't make much sense any more, especially after the change in this PR, where the basic example assumes that both online and batch serving must be present. Do you think we should have one single docker compose file instead, or keep the current setup?
There was a problem hiding this comment.
I like the idea of merging. I'll do that.
Done. |
|
/lgtm |
* Deduplicate example notebooks * Merge docker-compose.yml for both batch and online serving.
* Deduplicate example notebooks * Merge docker-compose.yml for both batch and online serving.
What this PR does / why we need it:
This PR removes the duplication of example notebooks between the docker-compose section and the root of the project. The docker compose file now spins up a complete Feast with the Feast repository mounted into it, and asks users to use the the examples in the
feast/examples/directory.Which issue(s) this PR fixes:
Fixes #415
Does this PR introduce a user-facing change?: