Miroslavsckaya/tg_rss_bot
1
1
Fork 0
Code Issues 7 Pull requests Projects 1 Releases Activity

Docker image #42

Merged
Miroslavsckaya merged 13 commits from feature_docker into master 2022-08-13 16:41:20 +00:00
Conversation 1 Commits 13 Files changed 6 +120 -3
6 changed files with 120 additions and 3 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

17
.dockerignore Normal file
View file

@ -0,0 +1,17 @@
# Python
.venv
__pycache__
# GIT
.gitignore
#CI configuration
.drone.yml
pylama.ini
# Bot documentation
README.md
# Environment
.env
skobkin marked this conversation as resolved Outdated
skobkin commented 2022-07-27 21:27:42 +00:00
Outdated
Review
Copy link

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.
.env.dist
skobkin marked this conversation as resolved Outdated
skobkin commented 2022-08-07 17:55:08 +00:00
Outdated
Review
Copy link

Missing EOL (\n)

Missing EOL (`\n`)

8
.env.dist Normal file
View file

@ -0,0 +1,8 @@
RSSBOT_TG_TOKEN=1234567890:yourbotstoken
skobkin marked this conversation as resolved Outdated
skobkin commented 2022-07-26 20:32:35 +00:00
Outdated
Review
Copy link

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

I'd add database and logging configuration examples here too, but leave them commented to show that they're optional.
# Optional variables
# RSSBOT_DSN=xxx
# POSTGRES_USER=xxx
# POSTGRES_PASSWORD=xxx
# POSTGRES_DB=xxx
# LOG_LEVEL=INFO
skobkin marked this conversation as resolved Outdated
skobkin commented 2022-07-27 21:30:42 +00:00
Outdated
Review
Copy link

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.

4
.gitignore vendored
View file

@ -5,5 +5,5 @@
/.venv
/__pycache__
# Database
/*.db
# Environment
skobkin marked this conversation as resolved
skobkin commented 2022-08-07 17:55:35 +00:00
Review
Copy link

Missing EOL (\n) on the last line

Missing EOL (`\n`) on the last line
.env
skobkin marked this conversation as resolved Outdated
skobkin commented 2022-07-26 20:33:01 +00:00
Outdated
Review
Copy link

Seems like you don't need that anymore.

Seems like you don't need that anymore.

21
Dockerfile Normal file
View file

@ -0,0 +1,21 @@
FROM python:3.10-alpine
skobkin marked this conversation as resolved Outdated
skobkin commented 2022-07-21 18:56:38 +00:00
Outdated
Review
Copy link

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)
WORKDIR /bot
COPY . .
skobkin marked this conversation as resolved
skobkin commented 2022-07-21 19:01:24 +00:00
Review
Copy link

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.
RUN pip install -r requirements.txt
ENV PYTHONUNBUFFERED=1
skobkin marked this conversation as resolved Outdated
skobkin commented 2022-07-21 19:02:46 +00:00
Outdated
Review
Copy link

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 Outdated
skobkin commented 2022-07-22 14:47:06 +00:00
Outdated
Review
Copy link

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

I'd recommend to also declare variables used by the app here. It can be done without specifying default value.
# App settings
# https://www.postgresql.org/docs/current/libpq-connect.html#LIBPQ-CONNSTRING
ENV RSSBOT_DSN=postgres://username:password@hostname/database_name
# https://core.telegram.org/bots#6-botfather
ENV RSSBOT_TG_TOKEN=1234567890:yourbotstoken
# https://docs.python.org/3/howto/logging.html#logging-levels
ENV LOG_LEVEL=INFO
ENTRYPOINT [ "python" ]
CMD [ "bot.py" ]

41
README.md
View file

@ -43,4 +43,43 @@ python bot.py
export RSSBOT_TG_TOKEN=xxx
export RSSBOT_DSN=xxx
python update.py
```
```
## Running prebuild Docker Image
### Running the bot
```shell
docker run -e RSSBOT_DSN=yyy RSSBOT_TG_TOKEN=xxx miroslavskaya/tg_rss_bot bot.py
```
### Running update
```shell
docker run -e RSSBOT_DSN=yyy RSSBOT_TG_TOKEN=xxx miroslavskaya/tg_rss_bot update.py
```
## Building and running Docker image from source
```shell
docker build . -t tg_rss_bot
```
### Running the bot
```shell
docker run -e RSSBOT_DSN=yyy RSSBOT_TG_TOKEN=xxx tg_rss_bot bot.py
```
### Running update
```shell
skobkin marked this conversation as resolved Outdated
skobkin commented 2022-08-07 17:56:02 +00:00
Outdated
Review
Copy link

Missing EOL (\n)

Missing EOL (`\n`)
docker run -e RSSBOT_DSN=yyy RSSBOT_TG_TOKEN=xxx tg_rss_bot update.py
```
## Using Docker Compose
### Running the bot
```shell
docker-compose up
```
### Running the update
```shell
docker-compose run app update.py
```

32
docker-compose.yml Normal file
View file

@ -0,0 +1,32 @@
version: '3.7'
skobkin marked this conversation as resolved
skobkin commented 2022-07-27 21:37:21 +00:00
Review
Copy link

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.
services:
app:
build: .
skobkin marked this conversation as resolved Outdated
skobkin commented 2022-07-26 20:35:02 +00:00
Outdated
Review
Copy link

Did you remove build on purpose?

Did you remove `build` on purpose?
image: miroslavsckaya/tg_rss_bot
environment:
# DSN schema: postgres://username:password@hostname/database_name
# https://www.postgresql.org/docs/current/libpq-connect.html#LIBPQ-CONNSTRING
skobkin marked this conversation as resolved Outdated
skobkin commented 2022-07-26 18:22:22 +00:00
Outdated
Review
Copy link

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
Outdated
Review
Copy link

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
Outdated
Review
Copy link

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.
- "RSSBOT_DSN=postgres://${BOT_DB_USER:-bot}:${BOT_DB_PASSWORD:-dev}@${BOT_DB_HOST:-db}/${BOT_DB_NAME:-bot}"
# https://core.telegram.org/bots#6-botfather
- "RSSBOT_TG_TOKEN=${RSSBOT_TG_TOKEN}"
# https://docs.python.org/3/howto/logging.html#logging-levels
- "LOG_LEVEL=${LOG_LEVEL:-INFO}"
skobkin marked this conversation as resolved Outdated
skobkin commented 2022-07-26 18:23:15 +00:00
Outdated
Review
Copy link

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

It's better to remove that if you're not sure that you need to have static container name without stack prefix.
depends_on:
- postgres
restart: unless-stopped
skobkin marked this conversation as resolved Outdated
skobkin commented 2022-07-26 18:25:59 +00:00
Outdated
Review
Copy link

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
Outdated
Review
Copy link

Not relevant. Default network is enough.

Not relevant. [Default network](https://docs.docker.com/compose/networking/) is enough.
db:
image: postgres:14-alpine
environment:
# Postgres settings
# https://hub.docker.com/_/postgres
- "POSTGRES_USER=${BOT_DB_USER:-bot}"
- "POSTGRES_PASSWORD=${BOT_DB_PASSWORD:-dev}"
- "POSTGRES_DB=${BOT_DB_NAME:-bot}"
volumes:
- db-data:/var/lib/postgresql/data
restart: unless-stopped
volumes:
skobkin marked this conversation as resolved
skobkin commented 2022-07-26 20:34:07 +00:00
Review
Copy link

db-data maybe?

`db-data` maybe?
db-data: