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

Improved docker setup #3940

Merged
merged 3 commits into from
Mar 21, 2024
Merged

Improved docker setup #3940

merged 3 commits into from
Mar 21, 2024

Conversation

wwelling
Copy link
Contributor

@wwelling wwelling commented Feb 6, 2024

VIVO GitHub issue: #3939

What does this pull request do?

This PR provides more configurability for docker compose and build of the VIVO docker image. Primarily addressing port mapping configuration via .env file for docker-compose.yml variable substitution.

What's new?

  1. solr and tomcat service port mapping configurable via .env
  2. ability to specify VIVO home path within the container
  3. local mount for VIVO solr core data to persist through container restarts

How should this be tested?

This can be tested by following https://github.com/wwelling/VIVO/blob/issue-3939/README.md#docker-compose. For more exhaustive testing the .env file can be modified between container restarts for desired effect.

Interested parties

@chenejac, @isl3

@wwelling wwelling linked an issue Feb 6, 2024 that may be closed by this pull request
Copy link
Member

@ivanmrsulja ivanmrsulja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM and works as intended! I left some comments that are open for discussion, if you have time, so please check them out 🙂

.env Show resolved Hide resolved
docker-compose.yml Outdated Show resolved Hide resolved
@isl3
Copy link

isl3 commented Feb 19, 2024

It might be helpful to some if there was a comment somewhere saying VIVO_HOST_PORT/SOLR_HOST_PORT can also specify host IP, but this should be obvious to anyone who knows how docker-compose files work so not an large concern.

@wwelling wwelling requested a review from ivanmrsulja February 20, 2024 02:28
Copy link
Member

@ivanmrsulja ivanmrsulja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks excellent now 🙂 great work!

Copy link
Contributor

@chenejac chenejac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wwelling well done. Thanks for this. I don't have any complaint, meaning from my point of view this is ready to be merged.

@litvinovg
Copy link
Collaborator

@chenejac @ivanmrsulja Can we write down more information about what was tested and reviewed?

@chenejac
Copy link
Contributor

@chenejac @ivanmrsulja Can we write down more information about what was tested and reviewed?

Tested by Ian Slatter, tested and reviewed by Ivan Mrsullja (linux), reviewed by Dragan Ivanovic.

@chenejac chenejac merged commit 602f706 into vivo-project:main Mar 21, 2024
4 checks passed
@wwelling wwelling mentioned this pull request Aug 22, 2024
@wwelling wwelling deleted the issue-3939 branch September 6, 2024 14:35
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

Successfully merging this pull request may close these issues.

Add bind address option to .env docker-compose
5 participants