assapin commented on a change in pull request #1:
URL: https://github.com/apache/incubator-liminal/pull/1#discussion_r524939950
##########
File path: scripts/liminal
##########
@@ -84,9 +84,10 @@ def deploy_liminal_core_internal(clean):
def docker_compose_command(command_name, args):
docker_compose_path, project_dir = get_docker_compose_paths()
concated_args = ' '.join(args)
- subprocess.call(
+ output = subprocess.run(
Review comment:
what happens when you do run on the command "up"? does it return?
##########
File path: scripts/liminal
##########
@@ -113,7 +124,7 @@ def start():
# initialize liminal home by default
environment.get_liminal_home()
docker_compose_path, project_dir = get_docker_compose_paths()
- docker_compose_command('up', [])
+ docker_compose_command('up', ['-d'])
Review comment:
-d means users don't by default see the logs.
it also enables the "stop" command to become available.
there is a question on how users see logs esp. errors.
I don't think it's ok to expect them to do docker ps and docker logs- some
of them don't know we're running docker.
I suggest to add liminal logs command which delegates to docker compose
logs. including the ability for -f and tail (delegate directly to docker
compose).
Also need to update the README with the stop and logs command, ideally also
add screenshots of outptus users should expect
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
