Skip to content
This repository has been archived by the owner on Mar 9, 2023. It is now read-only.

BaseURL Configuration for MarvinAI Notebook #32

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 11 additions & 8 deletions python-toolbox/marvin_python_toolbox/management/notebook.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,15 @@ def cli():

@cli.command('notebook', help='Start the Jupyter notebook server.')
@click.option('--port', '-p', default=8888, help='Jupyter server port')
@click.option('--base-url', default='/', help='Jupyter server base url')
@click.option('--enable-security', is_flag=True, help='Enable jupyter notebook token security.')
@click.option('--spark-conf', '-c', envvar='SPARK_CONF_DIR', type=click.Path(exists=True), help='Spark configuration folder path to be used in this session')
@click.option('--allow-root', is_flag=True, help='Run notebook from root user.')
@click.pass_context
def notebook_cli(ctx, port, enable_security, spark_conf, allow_root):
notebook(ctx, port, enable_security, spark_conf, allow_root)
def notebook_cli(ctx, port, base_url, enable_security, spark_conf, allow_root):
notebook(ctx, port, base_url, enable_security, spark_conf, allow_root)


def notebook(ctx, port, enable_security, spark_conf, allow_root):
def notebook(ctx, port, base_url, enable_security, spark_conf, allow_root):
notebookdir = os.path.join(ctx.obj['base_path'], 'notebooks')
command = [
"SPARK_CONF_DIR={0} YARN_CONF_DIR={0}".format(spark_conf if spark_conf else os.path.join(os.environ["SPARK_HOME"], "conf")),
Expand All @@ -49,6 +49,7 @@ def notebook(ctx, port, enable_security, spark_conf, allow_root):
'--config', os.path.join(os.environ["MARVIN_TOOLBOX_PATH"], 'extras', 'notebook_extensions', 'jupyter_notebook_config.py')
]

command.append("--NotebookApp.base_url=" + base_url)
command.append("--NotebookApp.token=") if not enable_security else None
command.append("--allow-root") if allow_root else None

Expand All @@ -58,14 +59,15 @@ def notebook(ctx, port, enable_security, spark_conf, allow_root):

@cli.command('lab', help='Start the JupyterLab server.')
@click.option('--port', '-p', default=8888, help='JupyterLab server port')
@click.option('--base-url', default='/', help='Jupyter server base url')
@click.option('--enable-security', is_flag=True, help='Enable jupyterlab token security.')
@click.option('--spark-conf', '-c', envvar='SPARK_CONF_DIR', type=click.Path(exists=True), help='Spark configuration folder path to be used in this session')
@click.pass_context
def lab_cli(ctx, port, enable_security, spark_conf):
lab(ctx, port, enable_security, spark_conf)
def lab_cli(ctx, port, base_url, enable_security, spark_conf):
lab(ctx, port, base_url, enable_security, spark_conf)


def lab(ctx, port, enable_security, spark_conf):
def lab(ctx, port, base_url, enable_security, spark_conf):
notebookdir = os.path.join(ctx.obj['base_path'], 'notebooks')
command = [
"SPARK_CONF_DIR={0} YARN_CONF_DIR={0}".format(spark_conf if spark_conf else os.path.join(os.environ["SPARK_HOME"], "conf")),
Expand All @@ -76,7 +78,8 @@ def lab(ctx, port, enable_security, spark_conf):
'--no-browser',
]

command.append("--NotebookApp.base_url=" + base_url)
command.append("--NotebookApp.token=") if not enable_security else None

ret = os.system(' '.join(command))
sys.exit(ret)
sys.exit(ret)
8 changes: 4 additions & 4 deletions python-toolbox/tests/management/test_notebook.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ def test_notebook(system_mocked, sys_mocked):
spark_conf = '/opt/spark/conf'
system_mocked.return_value = 1

notebook(ctx, port, enable_security, spark_conf, allow_root)
notebook(ctx, port, '/', enable_security, spark_conf, allow_root)
Copy link
Member

Choose a reason for hiding this comment

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

I think it is better to still create a variable called base_url instead of directly put a '/' in the code.


system_mocked.assert_called_once_with('SPARK_CONF_DIR=/opt/spark/conf YARN_CONF_DIR=/opt/spark/conf jupyter notebook --notebook-dir /tmp/notebooks --ip 0.0.0.0 --port 8888 --no-browser --config ' + os.environ["MARVIN_ENGINE_PATH"] + '/marvin_python_toolbox/extras/notebook_extensions/jupyter_notebook_config.py --NotebookApp.token=')

Expand All @@ -55,7 +55,7 @@ def test_notebook_with_security(system_mocked, sys_mocked):
spark_conf = '/opt/spark/conf'
system_mocked.return_value = 1

notebook(ctx, port, enable_security, spark_conf, allow_root)
notebook(ctx, port, '/', enable_security, spark_conf, allow_root)

system_mocked.assert_called_once_with('SPARK_CONF_DIR=/opt/spark/conf YARN_CONF_DIR=/opt/spark/conf jupyter notebook --notebook-dir /tmp/notebooks --ip 0.0.0.0 --port 8888 --no-browser --config ' + os.environ["MARVIN_ENGINE_PATH"] + '/marvin_python_toolbox/extras/notebook_extensions/jupyter_notebook_config.py')

Expand All @@ -69,7 +69,7 @@ def test_jupyter_lab(system_mocked, sys_mocked):
spark_conf = '/opt/spark/conf'
system_mocked.return_value = 1

lab(ctx, port, enable_security, spark_conf)
lab(ctx, port, '/', enable_security, spark_conf)

system_mocked.assert_called_once_with('SPARK_CONF_DIR=/opt/spark/conf YARN_CONF_DIR=/opt/spark/conf jupyter-lab --notebook-dir /tmp/notebooks --ip 0.0.0.0 --port 8888 --no-browser --NotebookApp.token=')
Copy link
Contributor

Choose a reason for hiding this comment

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

Adding '--NotebookApp.base_url=/' for assertion error fix.


Expand All @@ -83,6 +83,6 @@ def test_jupyter_lab_with_security(system_mocked, sys_mocked):
spark_conf = '/opt/spark/conf'
system_mocked.return_value = 1

lab(ctx, port, enable_security, spark_conf)
lab(ctx, port, '/', enable_security, spark_conf)

system_mocked.assert_called_once_with('SPARK_CONF_DIR=/opt/spark/conf YARN_CONF_DIR=/opt/spark/conf jupyter-lab --notebook-dir /tmp/notebooks --ip 0.0.0.0 --port 8888 --no-browser')
Copy link
Contributor

Choose a reason for hiding this comment

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

Adding '--NotebookApp.base_url=/' for assertion error fix.