Make data explorer non-blocking with docker compose #731
Conversation
|
The |
Makes sense indeed, it's always a bit clunky to stop it from the notebook. I'll update the PR |
Updated the PR, the explorer now uses docker compose to launch the service, slight change to the cli command: starting the service: Stopping the service: |
RobbeSneyders
changed the title
There was a problem hiding this comment.
Thanks @PhilippeMoussalli!
I had it a bit different in my mind, thinking about a docker compose file which we include in the Fondant package. But this approach indeed makes more sense so the compose file can be generated with different options 👍
The approach and code looks good to me, but I still want to test it. Will probably do so tomorrow!
There was a problem hiding this comment.
Just tested @PhilippeMoussalli. Works well, I would just catch errors with the docker command, see my comment below.
src/fondant/explore.py
Outdated
| "--detach", | ||
| ] | ||
|
|
||
| subprocess.call(cmd, stdout=subprocess.PIPE) # nosec |
There was a problem hiding this comment.
I think we should use check_call here so we don't print the messages below if this fails (eg. when the docker daemon is not running).
src/fondant/explore.py
Outdated
| try: | ||
| subprocess.check_call(cmd, stdout=subprocess.PIPE) # nosec | ||
| except subprocess.CalledProcessError as e: | ||
| raise OSError(e.returncode) |
There was a problem hiding this comment.
Why did you choose an OSError here? I would probably have gone for a SystemExit exception myself.
There was a problem hiding this comment.
Might make more sense here indeed, updated
Small PR to avoid showing the localhost url before the image is build