Fix: Conda, Apt-Get and Pip Install Plugins #594
Conversation
| @@ -26,6 +26,9 @@ class PluginManager: | |||
| tensorflow_on_spark=plugins.TensorflowOnSparkPlugin, | |||
| openblas=plugins.OpenBLASPlugin, | |||
| nvblas=plugins.NvBLASPlugin, | |||
| pip=plugins.PipPlugin, | |||
| apt_get=plugins.AptGetPlugin, | |||
There was a problem hiding this comment.
should we name this one "apt-get" or just "apt"?
There was a problem hiding this comment.
technically, since we are calling apt-get and not apt I think this is fine as is.
There was a problem hiding this comment.
I think we should call these apt_get_install, conda_install and pip_install since that is more indicative of what the plugin does.
There was a problem hiding this comment.
Though to compare, this is travis apt plugin https://docs.travis-ci.com/user/installing-dependencies/#Adding-APT-Packages
There was a problem hiding this comment.
This also has an option to update or not before
There was a problem hiding this comment.
I can't think of a case where you wouldn't want to update.
There was a problem hiding this comment.
Going to stick with the current convention of apt_get since the rest of the plugins are with '_'
| @@ -0,0 +1,8 @@ | |||
| #!/bin/bash | |||
|
|
|||
| apt-get update | |||
There was a problem hiding this comment.
shouldn't that only be for the apt?
There was a problem hiding this comment.
yeah, this is going to be refactored out. I think that having a single install.sh is fine so long as we do a check that $COMMAND is apt-get.
There was a problem hiding this comment.
I would mayabe have the full command be generted by the plugin and install.sh just eval it
There was a problem hiding this comment.
Yeah I'll move it to the full command
|
|
||
| for PACKAGE in "$@" | ||
| do | ||
| eval $COMMAND $PACKAGE |
There was a problem hiding this comment.
also for apt I think as @jafreck noticed it might be more performant to run apt install -y package1 package2, etc instead of multiple apt instance
(I think you should be able to do the same for pip and conda)
There was a problem hiding this comment.
Okay, I'll change it to a single instance
| PluginFile("install.sh", os.path.join(dir_path, "..", "install.sh")) | ||
| ], | ||
| args=packages, | ||
| env=dict(COMMAND='conda install') |
There was a problem hiding this comment.
Looks like the -y flag is necessary here to get around prompting. https://conda.io/docs/commands/conda-install.html
There was a problem hiding this comment.
WIll add the -y flag
brnleehng
changed the title
#545 (conda install plugin)
#546 (pip install plugin)
#547 (apt install plugin)