Docker image #42

Miroslavsckaya · 2022-07-21T18:46:52Z

Miroslavsckaya commented

2022-07-21 18:46:52 +00:00

No description provided.

Miroslavsckaya added 1 commit 2022-07-21 18:46:53 +00:00

add Dockerfile

continuous-integration/drone/push Build is passing

Details

continuous-integration/drone/pr Build is passing

Details

b8c91b351a

skobkin requested changes 2022-07-21 19:02:59 +00:00

Dockerfile Outdated

					
				@ -0,0 +1,13 @@

				FROM python:alpine

skobkin commented

2022-07-21 18:56:38 +00:00

It's better to fix our image to more specific image version.

As you're not installing any additional Alpine packages, you can mostly fix only Python version:

3.10-alpine (preferred, more stable, but will require manual update in the future to change minor Python version)
3-alpine (more flexible, but more likely to break over time)

It's better to fix our image to more specific image version. As you're not installing any additional Alpine packages, you can mostly fix only Python version: * `3.10-alpine` (preferred, more stable, but will require manual update in the future to change minor Python version) * `3-alpine` (more flexible, but more likely to break over time)

skobkin marked this conversation as resolved

Dockerfile

					
				@ -0,0 +2,4 @@

				WORKDIR /bot

				COPY . .

skobkin commented

2022-07-21 19:01:24 +00:00

You can probably copy only Python files and requirements.txt. We don't need anything else in the image to run the app.

You can use .dockerignore file to acomplish that. Check this documentation and my example.

This file will tell ADD/COPY to ignore some files you don't want to see in the image.

You can probably copy only Python files and `requirements.txt`. We don't need anything else in the image to run the app. You can use `.dockerignore` file to acomplish that. Check [this documentation](https://docs.docker.com/engine/reference/builder/#dockerignore-file) and [my example](https://git.skobk.in/skobkin/magnetico-web/src/branch/master/.dockerignore). This file will tell `ADD`/`COPY` to ignore some files you don't want to see in the image.

skobkin marked this conversation as resolved

Dockerfile Outdated

					
				@ -0,0 +6,4 @@

				RUN pip install -r requirements.txt

				ENV PYTHONUNBUFFERED='a'

skobkin commented

2022-07-21 19:02:46 +00:00

As it's not important what exactly is the value of this variable, it's better to use more intuitive value like 1 or TRUE.

By the way, AFAIR you can omit the quotes here.

As it's not important what exactly is the value of this variable, it's better to use more intuitive value like `1` or `TRUE`. By the way, AFAIR you can omit the quotes here.

skobkin marked this conversation as resolved

skobkin commented

2022-07-21 19:06:42 +00:00

Also don't forget to create Docker Compose stack for easy startup (and educational purposes).

Here some of my examples:

https://git.skobk.in/skobkin/docker-stacks

Also don't forget to create [Docker Compose](https://docs.docker.com/compose/) stack for easy startup (and educational purposes). Here some of my examples: https://git.skobk.in/skobkin/docker-stacks

Miroslavsckaya added 2 commits 2022-07-22 10:50:31 +00:00

add python version in Dockerfile and change PYTHONUNBUFFERED value to 1 dcba2e3b80

add .dockerignore

continuous-integration/drone/push Build is passing

Details

continuous-integration/drone/pr Build is passing

Details

2b4684275e

skobkin reviewed 2022-07-22 14:47:06 +00:00

Dockerfile Outdated

					
				@ -0,0 +7,4 @@

				RUN pip install -r requirements.txt

				ENV PYTHONUNBUFFERED=1

skobkin commented

2022-07-22 14:47:06 +00:00

I'd recommend to also declare variables used by the app here. It can be done without specifying default value.

skobkin marked this conversation as resolved

Miroslavsckaya added 1 commit 2022-07-25 19:47:40 +00:00

add other used environment variables

continuous-integration/drone/push Build is passing

Details

continuous-integration/drone/pr Build is passing

Details

6381661232

Miroslavsckaya added 1 commit 2022-07-25 20:29:19 +00:00

code style changes

continuous-integration/drone/push Build is passing

Details

continuous-integration/drone/pr Build is passing

Details

505e5f1ad5

skobkin approved these changes 2022-07-25 20:32:12 +00:00

skobkin left a comment

Looks nice.

Miroslavsckaya added 1 commit 2022-07-26 15:07:50 +00:00

add docker-compose.yml

continuous-integration/drone/push Build is passing

Details

continuous-integration/drone/pr Build is passing

Details

dcc7dda040

Miroslavsckaya added this to the RSS Bot Kanban Perdoling project 2022-07-26 15:16:43 +00:00

Miroslavsckaya added 1 commit 2022-07-26 16:38:50 +00:00

remove invalid values in docker-compose.yml and add little documentation for environment values

continuous-integration/drone/push Build is passing

Details

continuous-integration/drone/pr Build is passing

Details

a3327a505c

skobkin requested changes 2022-07-26 18:27:45 +00:00

skobkin left a comment

As you decided to also add Docker Compose configuration here, we now have some things to improve again 😆

docker-compose.yml Outdated

					
				@ -0,0 +6,4 @@

				    environment:

				      # DSN schema: postgres://username:password@hostname/database_name

				      # https://www.postgresql.org/docs/current/libpq-connect.html#LIBPQ-CONNSTRING

				      - "RSSBOT_DSN=${BOT_DSN}"

skobkin commented

2022-07-26 18:22:22 +00:00

If you're using substitution, then it'd be also good to provide an example file like .env.dist which could be simply copied to the .env file and quickly edited before the run.

If you're using substitution, then it'd be also good to provide an example file like `.env.dist` which could be simply copied to the `.env` file and quickly edited before the run.

skobkin commented

2022-07-26 18:22:29 +00:00

I also don't see why not to use the same variable name here to make .env file interoperable between direct python usage and Docker Compose.

I also don't see why not to use the same variable name here to make `.env` file interoperable between direct python usage and Docker Compose.

skobkin commented

2022-07-26 18:25:06 +00:00

If you're providing docker-compose.yml with PostgreSQL service included, you can at least configure the database and DSN out-of-the box here.

If you're providing `docker-compose.yml` with PostgreSQL service included, you can at least configure the database and DSN out-of-the box here.

skobkin marked this conversation as resolved

docker-compose.yml Outdated

					
				@ -0,0 +11,4 @@

				      - "RSSBOT_TG_TOKEN=${BOT_TOKEN}"

				      # https://docs.python.org/3/howto/logging.html#logging-levels

				      - "LOG_LEVEL=${BOT_LOG_LEVEL:-INFO}"

				    container_name: rss_bot

skobkin commented

2022-07-26 18:23:15 +00:00

It's better to remove that if you're not sure that you need to have static container name without stack prefix.

skobkin marked this conversation as resolved

docker-compose.yml Outdated

					
				@ -0,0 +15,4 @@

				    depends_on: 

				      - postgres

				  postgres:

skobkin commented

2022-07-26 18:25:59 +00:00

You've probably forgot to define a network to use in postgres and rss-bot services.

You've probably forgot to define a network to use in `postgres` and `rss-bot` services.

skobkin commented

2022-07-26 18:31:46 +00:00

Not relevant. Default network is enough.

Not relevant. [Default network](https://docs.docker.com/compose/networking/) is enough.

skobkin marked this conversation as resolved

skobkin added the

enhancement

new feature

labels 2022-07-26 18:28:17 +00:00

skobkin added this to the MVP 0.1 milestone 2022-07-26 18:28:21 +00:00

Miroslavsckaya was assigned by skobkin

2022-07-26 18:28:26 +00:00

skobkin removed this from the RSS Bot Kanban Perdoling project 2022-07-26 19:31:24 +00:00

Miroslavsckaya added 2 commits 2022-07-26 20:16:58 +00:00

add default values from environment variables 7e466a9384

add .env.dist

continuous-integration/drone/push Build is passing

Details

continuous-integration/drone/pr Build is passing

Details

90ac75fc11

skobkin requested changes 2022-07-26 20:36:51 +00:00

skobkin left a comment

Looks nice. Just a bit of polishing and let's merge it.

.env.dist Outdated

					
				@ -0,0 +1 @@

				RSSBOT_TG_TOKEN=1234567890:yourbotstoken

skobkin commented

2022-07-26 20:32:35 +00:00

I'd add database and logging configuration examples here too, but leave them commented to show that they're optional.

skobkin marked this conversation as resolved

.gitignore Outdated

					
				@ -7,3 +7,3 @@

				# Database

				/*.db

				/*.db

skobkin commented

2022-07-26 20:33:01 +00:00

Seems like you don't need that anymore.

skobkin marked this conversation as resolved

docker-compose.yml Outdated

					
				@ -0,0 +2,4 @@

				services:

				  app:

				    image: miroslavsckaya/tg_rss_bot

skobkin commented

2022-07-26 20:35:02 +00:00

Did you remove build on purpose?

Did you remove `build` on purpose?

skobkin marked this conversation as resolved

docker-compose.yml

					
				@ -0,0 +28,4 @@

				    restart: unless-stopped

				volumes:

				  postgres_bot_db:

skobkin commented

2022-07-26 20:34:07 +00:00

db-data maybe?

`db-data` maybe?

skobkin marked this conversation as resolved

Miroslavsckaya added 1 commit 2022-07-27 12:21:06 +00:00

add optional variables in .env.dist; remove *.db from .gitignore; add build command in docker-compose.yml

continuous-integration/drone/push Build is passing

Details

continuous-integration/drone/pr Build is passing

Details

8a82511618

Miroslavsckaya requested review from skobkin 2022-07-27 12:22:01 +00:00

skobkin approved these changes 2022-07-27 21:31:52 +00:00

skobkin left a comment

LGTM.
But I would've made a couple of small fixes here.

LGTM. But I would've made a couple of small fixes here.

.dockerignore Outdated

					
				@ -0,0 +13,4 @@

				README.md

				# Environment

				.env

skobkin commented

2022-07-27 21:27:42 +00:00

You forgot to also add .env.dist here 🙂
We definitely don't need it in the image.

You forgot to also add `.env.dist` here 🙂 We definitely don't need it in the image.

skobkin marked this conversation as resolved

.env.dist Outdated

					
				@ -0,0 +5,4 @@

				# POSTGRES_USER=xxx

				# POSTGRES_PASSWORD=xxx

				# POSTGRES_DB=xxx

				# LOG_LEVEL=xxx

skobkin commented

2022-07-27 21:30:42 +00:00

I'd add some default value for the LOG_LEVEL here because unlike Telegram credentials it can work out of the box and we already know all possible values.

I'd add some default value for the `LOG_LEVEL` here because unlike Telegram credentials it can work out of the box and we already know all possible values.

skobkin marked this conversation as resolved

skobkin reviewed 2022-07-27 21:37:21 +00:00

docker-compose.yml

					
				@ -0,0 +1,32 @@

				version: '3.7'

skobkin commented

2022-07-27 21:37:21 +00:00

I just understood that README.md has no documentation about running the bot in Docker yet.

I think it's a good idea to fix that.

I just understood that `README.md` has no documentation about running the bot in Docker yet. I think it's a good idea to fix that.

skobkin marked this conversation as resolved

Miroslavsckaya added 1 commit 2022-08-04 21:40:28 +00:00

add running the bot and runnig update with Docker documentation to README.md; add .env.dist to .dockerignore; set default value to LOG_LEVEL in .env.dist

continuous-integration/drone/push Build is passing

Details

continuous-integration/drone/pr Build is failing

Details

41b7eb32b6

skobkin reviewed 2022-08-07 17:55:08 +00:00

.dockerignore Outdated

					
				@ -0,0 +14,4 @@

				# Environment

				.env

				.env.dist

skobkin commented

2022-08-07 17:55:08 +00:00

Missing EOL (\n)

Missing EOL (`\n`)

skobkin marked this conversation as resolved

skobkin reviewed 2022-08-07 17:55:35 +00:00

.gitignore

					
				@ -7,3 +7,2 @@

				# Database

				/*.db

				# Environment

skobkin commented

2022-08-07 17:55:35 +00:00

Missing EOL (\n) on the last line

Missing EOL (`\n`) on the last line

skobkin marked this conversation as resolved

skobkin reviewed 2022-08-07 17:56:02 +00:00

README.md Outdated

					
				@ -46,1 +68,4 @@

				```shell

				docker-compose run app update.py

				```

skobkin commented

2022-08-07 17:56:02 +00:00

Missing EOL (\n)

Missing EOL (`\n`)

skobkin marked this conversation as resolved

skobkin referenced this pull request

2022-08-07 17:58:14 +00:00

Docker image CI build #43

skobkin added a new dependency 2022-08-07 17:58:26 +00:00

#43 Docker image CI build

Miroslavsckaya added 2 commits 2022-08-13 10:31:37 +00:00

add EOL to .gitignore, .dockerignor and README.md b9b12f21d4

change running and update documentation in README.md

continuous-integration/drone/push Build is passing

Details

continuous-integration/drone/pr Build is passing

Details

8c6c256971

skobkin approved these changes 2022-08-13 13:22:10 +00:00

skobkin left a comment

Let's merge it.

Miroslavsckaya changed title from ~~WIP:Docker image~~ to Docker image

2022-08-13 16:40:29 +00:00

Miroslavsckaya merged commit 3d40035f6b into master

2022-08-13 16:41:20 +00:00

Miroslavsckaya deleted branch feature_docker

2022-08-13 16:41:20 +00:00

Miroslavsckaya referenced this pull request from a commit

2022-08-13 16:41:21 +00:00

Docker image (#42)

Sign in to join this conversation.

No reviewers

No labels

No milestone

No project

No assignees

2 participants

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Blocks

#43 Docker image CI build

Miroslavsckaya/tg_rss_bot

Reference: Miroslavsckaya/tg_rss_bot#42

				`@ -0,0 +1,13 @@`
				`FROM python:alpine`

				`@ -0,0 +1 @@`
				`RSSBOT_TG_TOKEN=1234567890:yourbotstoken`

				`@ -0,0 +1,32 @@`
				`version: '3.7'`