-
Notifications
You must be signed in to change notification settings - Fork 66
Conversation
Can the script be a url? That would fix #309 as well. |
not as of now, not sure how we can make this one |
log.info("open rstudio server: %s%s", http_prefix, ssh_conf.rstudio_server_port) | ||
log.info("open jobhistoryui: %s%s", http_prefix, | ||
ssh_conf.job_history_ui_port) | ||
log.info("open namenodeui: %s%s", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should no longer be there, should be a plugin. All HDFS, Rstudio and Jupyter ports fall under the plugin category.
log.info("ssh username: %s", ssh_conf.username) | ||
log.info("connect: %s", ssh_conf.connect) | ||
log.info("-------------------------------------------") | ||
|
||
print_plugin_ports(cluster) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like there isn't a good reason to split plugins out into their own section like this, just print them like normal, but only show the ones that are actually installed on the cluster.
cli/config.py
Outdated
@@ -169,6 +169,12 @@ def cluster_config_from_dict(config: dict): | |||
if config.get('docker_repo') is not None: | |||
output.docker_repo = config['docker_repo'] | |||
|
|||
if 'plugins' in config: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not if config.get('plugins') is not None
as above?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved with a couple small UI suggestions. Also wondering if we should include information about plugins in cluster create
.
break | ||
|
||
if has_ports > 0: | ||
log.info("Plugins:") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like this should be lower case (just to match the text around it)
for plugin in plugins: | ||
for port in plugin.ports: | ||
if port.expose_publicly: | ||
log.info(" - open %s %s: %s%s", plugin.name, port.name or '', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we print format these so it's justified.
Plugins:
- open jupyter : http://localhost:8888
- open hdfs Namenode: http://localhost:50070
- open hdfs Datanodes: http://localhost:50075
to:
Plugins:
- open jupyter : http://localhost:8888
- open hdfs Namenode: http://localhost:50070
- open hdfs Datanodes: http://localhost:50075
fix #376
PluginDefinition
Docker image for plugins I think can come in v2 as all of this is already required
fix Verify custom scripts exist, exit gracefully if not #211 verify custom script exist
fix Feature: Plugins #387 (Custom ports)