Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changes to the plugin.py file to include the plugin remove_jar feature #116

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

nanaessuman
Copy link

I added "remove_jar" to plugin.jar to allow users to remove jars from directories when they add files with the add_jar feature.

@@ -47,3 +46,15 @@ def add_jar(local_path, plugin_name, plugin_dir=REMOTE_PLUGIN_DIR):
"""
_LOGGER.info('deploying jars on %s' % env.host)
write(local_path, os.path.join(plugin_dir, plugin_name))

def remove_jar(local_path, plugin_name):
Copy link
Member

Choose a reason for hiding this comment

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

For remove jar, there is no local path. The command should take the plugin name to remove and optionally the remote path to the plugin directory to allow for non-default configurations.

def remove_jar(plugin_name, plugin_dir=REMOTE_PLUGIN_DIR)

@rschlussel-zz
Copy link
Member

Thanks for the patch! We're excited to have a new contributor!
A couple notes on testing:
You should run 'make lint' to ensure that the code adheres to our style.
It's best to test with at least one presto node on a different node from presto-admin. Usually we write product tests to be able to run this automatically, but it's a bit complicated to set up, so it's fine if you just verify manually.

@kokosing
Copy link
Contributor

kokosing commented Mar 3, 2017

@nanaessuman Could you please address the comments from @rschlussel ? So we could merge this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants