Skip to content
This repository was archived by the owner on Jul 10, 2024. It is now read-only.

SUBMARINE-617. Update the apache/notebook:jupyter-notebook docker image#407

Closed
lowc1012 wants to merge 5 commits intoapache:masterfrom
lowc1012:SUBMARINE-617
Closed

SUBMARINE-617. Update the apache/notebook:jupyter-notebook docker image#407
lowc1012 wants to merge 5 commits intoapache:masterfrom
lowc1012:SUBMARINE-617

Conversation

@lowc1012
Copy link
Contributor

@lowc1012 lowc1012 commented Sep 21, 2020

What is this PR for?

  1. Update the apache/notebook:jupyter-notebook docker image
  2. Preinstall notebook & apache-submarine in jupyter notebook
  3. Set two environment variable (SUBMARINE_SERVER_DNS_NAME & SUBMARINE_SERVER_PORT) in notebook container for ease of internal communication between submarine-server and notebook.

What type of PR is it?

[Improvement]

Todos

  • - Task

What is the Jira issue?

SUBMARINE-617

How should this be tested?

Travis CI

Screenshots (if appropriate)

img1

img2

img3

img4

img5

Questions:

  • Does the licenses files need update? No
  • Is there breaking changes for older versions? No
  • Does this needs documentation? No

Copy link
Member

@jiwq jiwq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 If the comment by @pingsutw had resolved.

@lowc1012
Copy link
Contributor Author

I' m working on that inject submarine server DNS into notebook to easy the internal communication and SDK initialization. Please do not commit it temporarily. Thanks.

@lowc1012
Copy link
Contributor Author

In the first commit, apache-submarine package will be installed whenever creating notebook server. It spend a lot of time. I suggest that apache-submarine package should be installed before notebook container was running, and spend a lot of time pulling image when creating notebook server first time inevitably.
c.c. @jiwq @pingsutw

@lowc1012
Copy link
Contributor Author

@jiwq @pingsutw
I set two environment variables (SUBMARINE_SERVER_DNS_NAME and SUBMARINE_SERVER_PORT) in notebook container. Notebook can connect to submarine-server with Submarine SDK without entering the URL.
Please review this PR again. Thanks!

Copy link
Member

@pingsutw pingsutw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

other LGTM

@pingsutw
Copy link
Member

Could you also set the environment used by Jupyter Notebook in https://github.com/apache/submarine/blob/master/docs/database/submarine-data.sql?
So we don't need to manually create this environment

@lowc1012
Copy link
Contributor Author

@pingsutw Thanks for your review! I pushed a fixed-commit.

Copy link
Member

@pingsutw pingsutw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@xunliu xunliu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lowc1012 Thank you for contribution.
LGTM

@asfgit asfgit closed this in b47e43f Sep 26, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants