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

Feat postgres docker compose #68

Merged
merged 3 commits into from
Jul 8, 2024
Merged

Conversation

EduKav1813
Copy link
Collaborator

No description provided.

@EduKav1813 EduKav1813 requested a review from not7cd July 8, 2024 20:20
@EduKav1813 EduKav1813 self-assigned this Jul 8, 2024
@EduKav1813 EduKav1813 merged commit 1271840 into master Jul 8, 2024
1 check failed
Copy link
Member

@LiquidLemon LiquidLemon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kilka rzeczy mi się rzuciło w oczy, które można by troszeczkę poprawić.

- database:/data
- /etc/localtime:/etc/localtime:ro
restart: always
- ~/apps/postgres:/var/lib/postgresql/data
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Zazwyczaj do danych z bazy nie trzeba się dobijać bezpośrednio, więc tutaj mógłby być spokojnie named volume, ale jak to ma być zamontowany folder to lepiej jednak tworzyć go lokalnie w folderze, a nie w home'ie użytkownika. Wtedy nie ma ryzyka, że będzie konflikt z plikami z innego miejsca no i ktoś używający tego docker-compose nie będzie zaskoczony, że tworzą mu się pliki w jakichś dziwnych miejscach.

Comment on lines +8 to +23
db_dialect = os.environ.get("APP_DB_DIALECT", "sqlite")

if db_dialect == "sqlite":
db_path = os.environ.get("APP_DB_PATH", "whoisdevices.db")
db = pw.SqliteDatabase(db_path)
elif db_dialect == "postgresql":
db_name = os.environ.get("APP_DB_NAME", "whohacks")
db_user = os.environ.get("APP_DB_USER", "whohacks")
db_password = os.environ.get("APP_DB_PASSWORD")
db_host = os.environ.get("APP_DB_HOST", "localhost")
db_port = os.environ.get("APP_DB_PORT", "5432")
db = pw.PostgresqlDatabase(
db_name, user=db_user, password=db_password, host=db_host, port=db_port
)
else:
raise RuntimeError(f"Unknown db dialect '{db_dialect}' (envvar APP_DB_DIALECT)")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trzymanie poszczególnych informacji o połączeniu w różnych zmiennych środowiskowych jest trochę niewygodne, poza tym Peewee ma helpera do tworzenia obiektu bazy danych z URLa: http://docs.peewee-orm.com/en/latest/peewee/database.html#connecting-using-a-database-url

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dlaczego tylko niektóre zmienne środowiskowe mają dodany prefix APP_?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To w sumie wyszło ode mnie. Chciałem prosty sposób na wygenerowanie .env przy pomocy env.sh. Jest to bardzo przydatny pattern. Ale w tym przypadku ten prefix przy oauth więcej psuje. Więc prawdopodobnie trzeba będzie przywrócić poprzedni i zrobić jeden grep więcej w env.sh.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants