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
Cli: Stop containers #167
Cli: Stop containers #167
Conversation
@@ -108,7 +108,7 @@ def containerize(pre, force, install_js, stop): | |||
DockerHelper(cli_config.get_project_shortname(), local=False) | |||
) | |||
|
|||
commands.containerize(pre=pre, force=force, install=install_js, stop=stop) | |||
commands.containerize(pre=pre, force=force, install=install_js) |
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.
major: cannot comment on those lines. should remove stop
argument and click option, since it is not used anymore inside this command?
invenio_cli/cli.py
Outdated
@@ -156,6 +156,16 @@ def destroy(verbose): | |||
commands.destroy() | |||
|
|||
|
|||
@cli.command() | |||
@click.option('--verbose', default=False, is_flag=True, required=False, |
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.
moderate: This option is not used anywhere right? I think is a legacy from when we tried logging the output, might not be needed for now (I think is better to remove).
def stop(self): | ||
"""Stops containers.""" | ||
self.docker_helper.stop_containers() | ||
click.secho('Stopped containers', fg='green') |
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.
ultra-mega-ocd, feel free to ignore: All the actions in the other commands are being logged before happening (that way if something goes wrong the user knows where, is not just "a hung up call". Same applies to destroy below.
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 don't think I fully understood what you meant... Could you explain a bit further?
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.
In other commands (e.g. when destroying indices or whatever), the message is secho
ed before executing:
secho... # Tell the user it's gonna happen
docker_helper... # Make it happen
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.
Hope it is better now :)
Works as expected:
|
No description provided.