Add submit argument to allow skipping virtualenv requirement install #421
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dan-blanchard
requested changes
Mar 27, 2018
streamparse/cli/submit.py
Outdated
| @@ -161,7 +162,7 @@ def submit_topology(name=None, env_name=None, options=None, force=False, | |||
| use_venv = env_config.get('use_virtualenv', True) | |||
|
|
|||
| # Check if user wants to install virtualenv during the process | |||
| install_venv = env_config.get('install_virtualenv', use_venv) | |||
| install_venv = env_config.get('install_virtualenv', not skip_virtualenv) | |||
There was a problem hiding this comment.
A change that would be more consistent with our recent changes would be to change this line (and the use_venv line above it) to read
use_venv = options.get('use_virtualenv', True)
install_venv = options.get('install_virtualenv', use_venv)And put that after we call resolve_options.
That would let people specify sparse submit -o install_virtualenv=False to skip that installation without needing to create a new setting, or change any other code.
There was a problem hiding this comment.
@dan-blanchard Thanks for the excellent suggestion, this is way cleaner!
I've reverted my changes and implemented that in 0c33116, let me know if that looks good to you.
added 2 commits
March 27, 2018 18:25
…install" This reverts commit 3dc873e.
Instead of using only `env_config`, we use the resolved options which take into account options overwritten via CLI.
dan-blanchard
approved these changes
Mar 27, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This adds a CLI argument to the
submitcommand, allowing the user to skip installing virtualenv requirements.This is especially handy for situations where the virtualenv for a topology is already set up and the topology is getting resubmitted consecutively (e.g. for testing purposes).
This is already achievable via the
install_virtualenv: falseoption inconfig.json, but I think a CLI flag that controls that behaviour is useful too.I've only added it to the
submitcommand, and notupdate_virtualenv, as it's rather counter-intuitive to want to runupdate_virtualenvbut not install requirements.