Adding Dockerfile and docker compose files. #56
Conversation
To make it work with docker compose, I had to enable custom hostnames and ports for nats through environment vars. For this, I created a conf.py file in zambeze to centralize configuration code, such as pulling environment variables. #55
Attempt to fix GitHub action
|
Ok, I fixed the GitHub action. |
deployment/README.md
Outdated
| $> docker build -t zambeze/zambeze -f deployment/Dockerfile . | ||
| ``` |
There was a problem hiding this comment.
Nice, I didn't realize that "$>" was used to indicate a prompt I will use that from now on.
There was a problem hiding this comment.
Perhaps only the $ sign would be enough.
zambeze/settings.py
Outdated
| from .conf import HOST, NATS_HOST, NATS_PORT | ||
|
|
There was a problem hiding this comment.
Yep, I was unsure whether we should use it for ZMQ host. Now I think we should. See response: #56 (comment)
zambeze/conf.py
Outdated
| import os | ||
|
|
||
| HOST = os.getenv('HOST', "127.0.0.1") | ||
| NATS_HOST = os.getenv('NATS_HOST', "127.0.0.1") | ||
| NATS_PORT = int(os.getenv('NATS_PORT', 4222)) |
There was a problem hiding this comment.
Is conf.py the typical file name for a config file in Python? I don't know.
There was a problem hiding this comment.
Unfortunately there isn't a clear convention in Python. I've been using conf.py with my dev colleagues within all projects I develop, but after a quick search on other GitHub repos, we can see folks using config.py ; constants.py; and even configs.py. I'm fine with either file name. I just think it's nice to keep all global configuration constants, particularly the ones that come from env vars, in a single Python file.
| self.__set_default("host", NATS_HOST, self.settings["nats"]) | ||
| self.__set_default("port", NATS_PORT, self.settings["nats"]) |
There was a problem hiding this comment.
This is a lot clearer, no more magick numbers nice!
There was a problem hiding this comment.
We should do this with all of the ports and IP addresses.
zambeze/settings.py
Outdated
| self.__set_default("host", "127.0.0.1", self.settings["nats"]) | ||
| self.__set_default("port", 4222, self.settings["nats"]) | ||
| self.__set_default("host", NATS_HOST, self.settings["nats"]) | ||
| self.__set_default("port", NATS_PORT, self.settings["nats"]) | ||
| self.__set_default("host", "127.0.0.1", self.settings["zmq"]) |
There was a problem hiding this comment.
Did you mean to use HOST here?
There was a problem hiding this comment.
I meant NATS_HOST for line 80. For line 82, I think HOST is right. I just wasn't sure whether the zmq will always run on the same host as the agent. I so, then yes we should use HOST in line 82 and never have something like ZEROMQ_HOST as an env var.
| version: '3.8' | ||
| services: | ||
| nats: | ||
| container_name: nats | ||
| image: nats | ||
| command: -V | ||
| ports: | ||
| - 4222:4222 | ||
| - 6222:6222 | ||
| - 8222:8222 |
There was a problem hiding this comment.
I'm asking cause I don't know, where does this container come from? Is this provided by nats registry?
There was a problem hiding this comment.
This is exactly right. See: https://hub.docker.com/_/nats
deployment/README.md
Outdated
|
|
||
|
|
||
| ```shell | ||
| $> docker build -t zambeze/zambeze -f deployment/Dockerfile . |
There was a problem hiding this comment.
Question, do you think it is best to place this in a deployment folder or a different folder name i.e. like development, compose or something else, this is not really a problem, just brainstorming. Deployment to me makes me think of production.
There was a problem hiding this comment.
Again it's been a convention within my former colleagues to always place Dockerfiles, compose files, k8s files, etc etc under a folder called deployment. I'm not sure whether there's a different convention and we can definitely change/create our own.
|
@par-hermes format |
|
If the checks are failing because of formatting I would just ignore them, and merge anyway specifically if you have already run the auto formatted. Stupid formatting errors that are not auto fixable are a waste of time for the most part. |
Please teach me later how to identify formatting build failures and how to fix them. I'm just not used to adding them in CI/CD pipelines, but think it's a good idea to keep them. |
- Using HOST env var for zmq, and created a ZMQ_PORT var. - Improved README.md for the container execution to provide the steps to run the simple example and expected results. - Made few other code improvements based on @JoshuaSBrown's feedback on this PR: #56 Issue: #55
To make it work with docker compose, I had to enable custom hostnames and ports
for nats through environment vars. For this, I created a conf.py file in
zambeze to centralize configuration code, such as pulling environment variables.
#55