Skip to content
This repository has been archived by the owner on Feb 3, 2021. It is now read-only.

Feature: Plugins #387

Merged
merged 77 commits into from
Feb 27, 2018
Merged

Feature: Plugins #387

merged 77 commits into from
Feb 27, 2018

Conversation

timotheeguerin
Copy link
Member

@timotheeguerin timotheeguerin commented Feb 8, 2018

fix #376
image

image

  • Create a plugin definition
  • Reference plugin in the code
  • Plugin define script to run with arguments
  • Plugin define port to open
  • SSH automatically open port defined in plugin
  • Validate arguments
  • Validate plugin definition returns PluginDefinition
  • Optional name for ports if multiple
  • Move all custom scripts to plugin
  • Write tests
  • Make plugins log in different file
  • List plugins api

Docker image for plugins I think can come in v2 as all of this is already required

@jafreck
Copy link
Member

jafreck commented Feb 8, 2018

Can the script be a url? That would fix #309 as well.

@timotheeguerin
Copy link
Member Author

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",
Copy link
Member

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)
Copy link
Member

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:
Copy link
Member

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?

@jafreck jafreck modified the milestones: v0.8.0, v0.7.0 Feb 26, 2018
Copy link
Member

@jafreck jafreck left a 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:")
Copy link
Member

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 '',
Copy link
Member

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

@timotheeguerin timotheeguerin merged commit c724d94 into master Feb 27, 2018
@timotheeguerin timotheeguerin deleted the feature/plugins branch April 5, 2018 16:31
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Plugins Verify custom scripts exist, exit gracefully if not
3 participants