-
Notifications
You must be signed in to change notification settings - Fork 4
add csv option to data dump script #534
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
base: master
Are you sure you want to change the base?
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #534 +/- ##
=======================================
Coverage 97.66% 97.66%
=======================================
Files 214 214
Lines 12438 12438
=======================================
Hits 12147 12147
Misses 291 291 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
|
||
| # Stage 2 - build frontend | ||
| FROM node:24-bookworm-slim AS frontend-build | ||
| FROM node:24-trixie-slim AS frontend-build |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this one needs to be updated in the Objects API dockerfile as well: https://github.com/maykinmedia/objects-api/blob/master/Dockerfile#L23
|
|
||
| # | ||
| # with --csv a csv dump can be created for all tables in the given components. The csv files will be generated in the temporary directory csv_dumps | ||
| # and combined into a single TAR archive csv_dumps. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I noticed that /dump_data.sh in this repo runs without prompting for DB passwords, while in Objects it does prompt for a password. Is it possible to have the same behavior in Objects?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated the settings and compose example to make it more inline with objects but the script does use the DB_PASSWORD env var so if that is added to the compose it will not prompt.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah I meant to make Objects more in line with this PR, because the container needs the DB_USER, DB_PASSWORD envvars even if you don't use the script, right?
Currently if I try to bring up the containers locally I get
db-1 | 2025-11-11 10:33:29.823 UTC [64] LOG: database system was shut down at 2025-11-11 10:33:29 UTC
db-1 | 2025-11-11 10:33:29.828 UTC [1] LOG: database system is ready to accept connections
web-init-1 | db:5432 - accepting connections
web-init-1 | Database is up.
web-init-1 | /app/src/openklant/celery.py:14: RuntimeWarning: No OTEL_SERVICE_NAME environment variable set, using a default. You should set a (distinct) value for each component (web, worker...)
web-init-1 | setup_env()
web-init-1 | {"event": "error connecting in 'pool-1': connection failed: connection to server at \"172.18.0.3\", port 5432 failed: fe_sendauth: no password supplied", "timestamp": "2025-11-11T10:33:31.177079Z", "logger": "psycopg.pool", "level": "warning"}
Does this occur for you as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it should work if you delete the container and volume i think (docker compose down db -v)
DB_PASSWORD isn't needed in the compose examples because it just uses the default password from conf/docker.py which is the project name in most cases.
So should i add DB_PASSWORD to all compose examples so that dump_data has the password?
Fixes maykinmedia/open-api-framework#188
Changes
--csvoption to data dump script