demo notebook fixes for EMR - #1093
Conversation
Signed-off-by: Oleg Avdeev <oleg.v.avdeev@gmail.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: oavdeev The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
/kind housekeeping |
|
@oavdeev: The following test failed, say
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. I understand the commands that are listed here. |
| "outputs": [], | ||
| "source": [ | ||
| "KAFKA_BROKER = \"kafka:9092\"" | ||
| "KAFKA_BROKER = os.getenv(\"DEMO_KAFKA_BROKERS\", \"kafka:9092\")" |
There was a problem hiding this comment.
Would it make sense to hoist these variables to the top so that we know which env vars to set outside the notebook?
| "SERVING_HOST = os.getenv(\"DEMO_FEAST_ONLINE_SERVING_SERVICE_HOST\", \"localhost\")\n", | ||
| "REDIS_HOST = os.getenv('DEMO_REDIS_MASTER_SERVICE_HOST', 'localhost')\n", | ||
| "REDIS_HOST = os.getenv('DEMO_REDIS_MASTER_SERVICE_HOST', os.getenv('FEAST_REDIS_HOST'))\n", | ||
| "DEMO_DATA_LOCATION = os.getenv(\"DEMO_DATA_LOCATION\")\n", |
There was a problem hiding this comment.
Should we rename this from demo to something more generic? demo seems specific to our demos whereas this notebook is version controlled for use as a tutorial.
|
I'm gonna close this and open a new PR, it was easier to create a new branch than rebase this |
What this PR does / why we need it:
A few tweaks to the demo notebook to make it work on EMR, while hopefully not breaking GCP support either
Which issue(s) this PR fixes:
Fixes #
Does this PR introduce a user-facing change?: