Conversation
|
@nuclearcat I want to confirm too that you have the Also, is there any other change to be done? |
tales-aparecida
left a comment
There was a problem hiding this comment.
lgtm
I would appreciate including "why" in the commit message bodies, especially on the one separating the ingester container
41494d9 to
db46b2a
Compare
| \"NAME\": \"${DB_DEFAULT_NAME:=kcidb}\", | ||
| \"USER\": \"${DB_DEFAULT_USER:=kernelci}\", | ||
| \"PASSWORD\": \"$DB_DEFAULT_PASSWORD\", | ||
| \"HOST\": \"${DB_DEFAULT_HOST:=cloudsql-proxy}\", |
There was a problem hiding this comment.
wouldn't be needed to change DB_DEFAULT_NAME and DB_DEFAULT_USER to match de dashboard_db ones?
.env.db.example sets:
POSTGRES_USER=${DASH_DB_USER:-admin}
POSTGRES_PASSWORD=${DASH_DB_PASSWORD:-admin}
POSTGRES_DB=${DASH_DB_NAME:-dashboard}
There was a problem hiding this comment.
I had left them different since DB_DEFAULT_ is supposed to be a different db from DASH_DB_, but I guess it's ok to point both to the local db
db46b2a to
e939f1c
Compare
If the ingester was initialized with an empty trees_file argument it was not defaulting correctly since None was passed to the function
e939f1c to
08c5b62
Compare
|
Added changes for fixing the exit of the ingester service on docker, and also adding the execution of the other required command |
08c5b62 to
f3579df
Compare
The ingester is used to insert data into the database, so we always want it running smoothly in the project while also not consuming resources or having its resources consumed by the backend as if it were just a cron job Also fixes signal handling when running the ingester command within docker and avoids running cronjobs in the new service. Co-authored-by: Marcelo Robert Santos <marcelo.santos@profusion.mobi>
Follows the addition of the ingester service. This new service will consume the items generated by the ingester aggregation and organize them into the listing tables. It is done as a new service instead of a cronjob in order to avoid over-execution and multiple processes being started Also adds a dockerignore file to avoid building cache folder/files and the folder for submission testing
Removed old configuration for Google Cloud because it has been replaced with Azure database Co-authored-by: Marcelo Robert Santos <marcelo.santos@profusion.mobi>
f3579df to
721ab74
Compare
gustavobtflores
left a comment
There was a problem hiding this comment.
Looks good. I'd like to see trees.yaml generated automatically inside the container, it would make onboarding and external contributions much easier
Absolutely! We need to avoid dropping anything manually. |
Replaces the use of a cronjob to update the tree-names file with updating it with the ingester. This also lets the command be called within the code.
|
@nuclearcat @gustavobtflores Added a change to update the trees file when the ingester is run, this is also possible because that's currently the only reason we're still using this file. All changes are in the last commit |
Changes
process_pending_aggregationSince there are profiles in the new containers, by running
docker compose upthey won't be started by default. But if you rundocker compose --profile with-commands upyou'll run all containers including the new services.I wasn't able to connect to the production database through docker, so the instructions reflect that.
Closes #1721