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

Healthchecks and port mappings for docker-compose #87

Merged

Conversation

@dangarthwaite
Copy link
Contributor

commented Oct 10, 2019

No description provided.

@dangarthwaite

This comment has been minimized.

Copy link
Contributor Author

commented Oct 10, 2019

For #59

Copy link
Collaborator

left a comment

This looks as though it will lose a lot of the features, however your comment in #59 is right it will improve readability significantly.

The idea is that the configuration uses one replacement mechanism to find the source configuration string and replaces it but this said I have not kept up to date on the default state of the Chainquery configuration in a while so this may be the best method to take at this point.

The goal with my previous start.sh script configuration was to allow people to lean on the docker container via environment variables as that seems to be the most multi container platform compatible solution.

All this said upon my review of my own goals here I'm seeing a few flaws in what I decided on using here as well so as long as we don't lose too much in this PR I will gladly merge it.

Thank you for your contributions!

@dangarthwaite

This comment has been minimized.

Copy link
Contributor Author

commented Oct 10, 2019

Copy link
Collaborator

left a comment

Just looking for replies on the comments, likely 100% acceptable just want to follow up.

Thank you for your PR's

@@ -8,16 +8,13 @@ services:
###########
## MYSQL ##
###########
## MariaDB is currently not supported and neither is later versions of MySQL this may change.
## https://hub.docker.com/r/_/mariadb/
## Work is underway to support percona

This comment has been minimized.

Copy link
@Leopere

Leopere Oct 10, 2019

Collaborator

Has this been decided proper at this point I spoke with @nikooo777 a month or so ago and was told not to bother.

This comment has been minimized.

Copy link
@Leopere

Leopere Oct 10, 2019

Collaborator

Also thank you for the cleanup.

healthcheck:
test: ["CMD-SHELL", 'mysql --database=$$MYSQL_DATABASE --password=$$MYSQL_ROOT_PASSWORD --execute="SELECT count(table_name) > 0 FROM information_schema.tables;" --skip-column-names -B']
interval: 5s
retries: 12
Comment for lines +26  – +29

This comment has been minimized.

Copy link
@Leopere

Leopere Oct 10, 2019

Collaborator

Looks awesome!

build:
context: .
dockerfile: Dockerfile-linux-x86_64-production
Comment for lines +39  – +41

This comment has been minimized.

Copy link
@Leopere

Leopere Oct 10, 2019

Collaborator

Would we prefer to build the production example rather than pull from the LBRY official repo?

restart: always
networks:
lbry-network:
ipv4_address: 10.6.1.3
env_file:
- env
- ../environment/lbrycrd.env
- ../environment/chainquery.env

This comment has been minimized.

Copy link
@Leopere

Leopere Oct 10, 2019

Collaborator

This is likely fine but I might've been just pulling the lbrycrd env file due to only needing to know the RPC credentials.

@@ -3,22 +3,24 @@
FROM ubuntu:18.04 as prep
LABEL MAINTAINER="leopere [at] nixc [dot] us"
RUN apt-get update && \
apt-get -y install unzip curl && \
apt-get -y install unzip curl busybox-static && \

This comment has been minimized.

Copy link
@Leopere

Leopere Oct 10, 2019

Collaborator

Cool trick, I like this!

@lbry-bot lbry-bot assigned Leopere and unassigned Leopere Oct 10, 2019
@Leopere Leopere added this to the Needs milestone Oct 10, 2019
Copy link
Collaborator

left a comment

Thanks for your contributions.

@lbry-bot lbry-bot assigned Leopere and unassigned Leopere Oct 10, 2019
@Leopere Leopere merged commit fced928 into lbryio:master Oct 10, 2019
1 check passed
1 check passed
ci/dockercloud (/chainquery:/Dockerfile-linux-x86_64-production) Your tests passed in Docker Cloud
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.