galaxy-genome-annotation / docker-tripal

Docker container for Tripal
10 stars 11 forks source link

Upgrade docker-compose.yml to v3 and network spec to 'best practice' #29

Closed RichardBruskiewich closed 5 years ago

RichardBruskiewich commented 5 years ago

The current docker-tripal docker-compose.yml file uses the "links" directive for networking, which is somewhat deprecated (see https://docs.docker.com/compose/compose-file/#links). This pull request updates the docker-compose.yml to comply with the recommendation to use the "networks" directives instead. Along the way, the docker-compose.yml version is set to version "3". The web "depends_on" directive was added for good measure to encourage proper sequencing in the build deployment.

Testing this revised docker-compose.yml file, it seems to still work fine with the new directives.

abretaud commented 5 years ago

Thanks @RichardBruskiewich! Why not rename the db container to 'postgres'? It would allow to get rid of the network definition as bridge is already the default mode

RichardBruskiewich commented 5 years ago

@abretaud sure, fine by me. I'll do that and retest. The point is that the "links" specification is deprecated. A simpler (default) network specification makes sense here.

abretaud commented 5 years ago

@abretaud sure, fine by me. I'll do that and retest. The point is that the "links" specification is deprecated. A simpler (default) network specification makes sense here.

Yes, I'm fine with replacing links by depends_on, it's just that defining the network and specifying it would not be needed if the db container was just renamed postgres. I like to keep only necessary code ;)

RichardBruskiewich commented 5 years ago

Hi Anthony,

see my latest commit to my fork of v3.x. I've taken all of your advice - renamed 'db' to 'postgres' and removing explicit networking since it is "bridge implied" using service tags as Docker network hostnames of the containers. The "depends_on" is more of a container startup sequencing dependency directive (not related to networking). I did test a comparable build - works fine. Over to you to process the pull request now!

abretaud commented 5 years ago

Thank you @RichardBruskiewich !