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

Describe BOTH methods of starting the container in the README #379

Closed
CyMule opened this issue Jan 19, 2023 · 3 comments
Closed

Describe BOTH methods of starting the container in the README #379

CyMule opened this issue Jan 19, 2023 · 3 comments

Comments

@CyMule
Copy link
Contributor

CyMule commented Jan 19, 2023

Suggestion

Describe BOTH methods of starting the container in the README.

Reasoning

Its nice to have options, and I wasn't initially familiar with the docker compose -f docker-compose.yml -f docker-compose.local.yml up syntax. I've used docker compose up many times, but I had to do a quick google to understand the override syntax.

Method 1

docker compose up

In the current configuration, docker compose up works and it uses named volumes:
docker-compose.yml

...
    volumes:
      - unchained:/unchained
      - cache:/cache
volumes:
  unchained:
  cache:

Method 2

The second method is docker compose -f docker-compose.yml -f docker-compose.local.yml up which is what is currently described in the README.

This mounts folders from the host machine into the container.

Questions

Is method 2 the preferred method for starting the container? As in do you prefer having the ability to access the folders on the host machine?

If so we could reverse the configuration so that the default start command would be docker compose up and then if you wanted named volumes INSTEAD of the mounted folders you would use the longer command docker compose -f docker-compose.yml -f docker-compose.local.yml up .

This would then require swapping the volume parts of the two file so
docker-compose.yml would be:

name: TrueBlocksCore

services:
  core:
    image: trueblocks/core:v0.50.0-beta
    build: ./
    env_file: .env
    ports:
      - "8080:${SERVE_PORT-8080}"
    volumes:
      - type: bind
        source: /Users/user/Data/cache
        target: /cache
      - type: bind
        source: /Users/user/Data/unchained
        target: /unchained

and docker-compose.local.yml would be:

services:
  core:
    volumes:
      - unchained:/unchained
      - cache:/cache
volumes:
  unchained:
  cache:

Keeping the docker-compose.yml as is means that docker compose up works with no additional work. Using the mounted folders requires the user to create the folders prior as instructed which obviously isn't a huge step.

@tjayrush
Copy link
Member

Is method 2 the preferred method for starting the container?

I think so (but I'm open to discussion). Reasons why:

  • The end user can install and run the docker version to scrape only and not run the API server. He/she may do this because they want to use the command line tools (which are slightly faster than the API for some tools). If the folders were not exposed to the external machine, then either this type of user would have to have duplicate data (one inside and one outside the docker) or not use the docker.
  • Another type of use may want to run against the API server. In this case, he/she would want all the files internal, but there would be a speed tradeoff.
  • Downside of not having the files outside of the docker contain is the weird way one must use the chifra.sh script.

On the upside of having it entirely inside the docker container:

  • Cleaner
  • Easier to remove from the machine
  • Other?

@tjayrush
Copy link
Member

tjayrush commented Jan 23, 2023

If we do decide to suggest an alternative way to run the docker version, I would suggest we have a section in the README called something like "An Alternative Way to Start the Docker" This as opposed to trying to intertwine the instructions of the two methods in the same section, which I think will confuse both methods.

Plus having these two different modes in separate sections implicitly shows the user that one method is preferred.

@CyMule
Copy link
Contributor Author

CyMule commented Feb 4, 2023

I also prefer method 2 (making the folders first) instead of using the docker volumes. This is partially based on the assumption that new users will start might start with the docker version and then transition to using chifra directly rather than through docker.
The transition is easier with method 2.

CyMule added a commit to CyMule/trueblocks-docker that referenced this issue Feb 4, 2023
Previously `docker compose up` would use docker volumes. This
pull request requires users to first make 2 folders, one for
cache and one for unchained.

The override file now allows users to use docker volumes where
it previously mounted the users 2 host files for the
cache/unchained storage.

Closes TrueBlocks#379
CyMule added a commit to CyMule/trueblocks-docker that referenced this issue Feb 4, 2023
Previously `docker compose up` would use docker volumes. This
pull request requires users to first make 2 folders, one for
cache and one for unchained.

The override file now allows users to use docker volumes where
it previously mounted the users 2 host files for the
cache/unchained storage.

Closes TrueBlocks#379
CyMule added a commit to CyMule/trueblocks-docker that referenced this issue Feb 4, 2023
Previously `docker compose up` would use docker volumes. This
pull request requires users to first make 2 folders, one for
cache and one for unchained.

The override file now allows users to use docker volumes where
it previously mounted the users 2 host files for the
cache/unchained storage.

Closes TrueBlocks#379
CyMule added a commit to CyMule/trueblocks-docker that referenced this issue Feb 4, 2023
Previously `docker compose up` would use docker volumes. This
pull request requires users to first make 2 folders, one for
cache and one for unchained.

The override file now allows users to use docker volumes where
it previously mounted the users 2 host files for the
cache/unchained storage.

Closes TrueBlocks#379
CyMule added a commit to CyMule/trueblocks-docker that referenced this issue Feb 4, 2023
Previously `docker compose up` would use docker volumes. This
pull request requires users to first make 2 folders, one for
cache and one for unchained.

The override file now allows users to use docker volumes where
it previously mounted the users 2 host files for the
cache/unchained storage.

Closes TrueBlocks#379
CyMule added a commit to CyMule/trueblocks-docker that referenced this issue Feb 4, 2023
Previously `docker compose up` would use docker volumes. This
pull request requires users to first make 2 folders, one for
cache and one for unchained.

The override file now allows users to use docker volumes where
it previously mounted the users 2 host files for the
cache/unchained storage.

`docker-compose.local.example` is renamed to `docker-compose.volume-override.yml`
because of the previosuly stated changes.

`scripts/up.sh` is deleted because the command `docker compose up`
is now the primary command used instead of the longer override command.

Closes TrueBlocks#379
tjayrush pushed a commit that referenced this issue Feb 6, 2023
Previously `docker compose up` would use docker volumes. This
pull request requires users to first make 2 folders, one for
cache and one for unchained.

The override file now allows users to use docker volumes where
it previously mounted the users 2 host files for the
cache/unchained storage.

`docker-compose.local.example` is renamed to `docker-compose.volume-override.yml`
because of the previosuly stated changes.

`scripts/up.sh` is deleted because the command `docker compose up`
is now the primary command used instead of the longer override command.

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

No branches or pull requests

2 participants