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

Simplify Quickstart #281

Merged

Conversation

ckadner
Copy link
Member

@ckadner ckadner commented Dec 2, 2021

Proposed Changes:

  • Since we have /catalog/upload_from_url API now (Added upload-from-url to catalog service #259), we no longer need to keep a copy of the catalog_upload.json file under quickstart
  • Since all assets get published and featured automatically with catalog upload (Feature all assets from catalog upload #256), there is no longer a need for the init_catalog.sh script either
  • Since the MLX API also creates the Pipelines table (3c10fda) we no longer need to mount the init_db.sql to the mysql service
  • Since the Docker Compose File v3+ spec no longer supports condition as an option of depends_on we no longer need the healthcheck on services, but instead have to use simple while-sleep-loops in the commands of downstream services

Another benefit of this change is, that first time users no longer have to clone the machine-learning-exchange/mlx Github repo in order to run the Quickstart:

These two lines are sufficient:

wget https://raw.githubusercontent.com/ckadner/mlx/simplify_quickstart/quickstart/docker-compose.yaml
docker compose up

Related Issues:

FYI @girigsrini -- these are the changes under quickstart after you rework your PR #262 to address the changes under bootstrapper

* Since we have /catalog/upload_from_url API now (machine-learning-exchange#259), we
  no longer need to keep a copy of the catalog_upload.json
  file under quickstart
* Since all assets get published and featured automatically
  with catalog upload (machine-learning-exchange#256), there is no longer a need for
  the init_catalog.sh script either
* Since the MLX API also creates the Pipelines table (3c10fda)
  we no longer need to mount the init_db.sql to the mysql
  service
* Since the Docker Compose File v3+ spec no longer supports
  condition as an option of depends_on we no longer need the
  healthcheck on services, but instead have to use simple
  while-wait-loops in the commands of downstream services

Related machine-learning-exchange#256
Related machine-learning-exchange#259
Related 3c10fda
Related https://forums.docker.com/t/depends-on-version-3/29463

Signed-off-by: Christian Kadner <[email protected]>
@mlx-bot
Copy link
Collaborator

mlx-bot commented Dec 2, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ckadner

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ckadner ckadner removed the request for review from drewbutlerbb4 December 2, 2021 21:18
@ckadner
Copy link
Member Author

ckadner commented Dec 3, 2021

Besides simplifying the docker-compose.yaml file and removing duplicated files, another benefit of this change is that first time users no longer have to clone the machine-learning-exchange/mlx Github repo in order to run the Quickstart:

These two lines are sufficient:

wget https://raw.githubusercontent.com/ckadner/mlx/simplify_quickstart/quickstart/docker-compose.yaml
docker compose up

@Tomcli
Copy link
Member

Tomcli commented Dec 3, 2021

/lgtm

@mlx-bot mlx-bot added the lgtm label Dec 3, 2021
@mlx-bot mlx-bot merged commit 2a1f2ea into machine-learning-exchange:main Dec 3, 2021
@ckadner
Copy link
Member Author

ckadner commented Dec 3, 2021

Thanks @Tomcli

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

Successfully merging this pull request may close these issues.

3 participants