medic / cht-sync

Data synchronization between CouchDB and PostgreSQL for the purpose of analytics.
GNU General Public License v3.0
4 stars 5 forks source link

feat(#174): bastion Dockerfile and compose file #177

Open mrjones-plip opened 1 month ago

mrjones-plip commented 1 month ago

Description

This PR:

closes #174

Code review checklist

License

The software is provided under AGPL-3.0. Contributions to this project are accepted under the same license.

mrjones-plip commented 4 weeks ago

@witash and @njuguna-n - is it OK to remove these two lines that expose the postgres ports ? for a sane default for production I don't think we want this exposed - I've removed it in this PR.

Related: For development I think we should use pgadmin. I also think we should break out pgadmin to it's own docker file so it's not run by default in production. - I went ahead and did this while I was in there!

Thought: Instead of docker-compose.pgadmin.yml, we could call this docker-compose.developer-postgres.yml or something. It would not only add pgadmin service but also expose 5432 for postgres server defined in docker-compose.postgres.yml. This is nice b/c you can easily run it in development and it's clear what it's point is. I think most production instances should not expose pgadmin on 5050 by default - but maybe I'm taking it too far?

services:
  postgres:
    ports:
      - 5432:5432

  pgadmin:
    image: dpage/pgadmin4
    environment:
      PGADMIN_DEFAULT_EMAIL: ${PGADMIN_DEFAULT_EMAIL:-pgadmin4@pgadmin.org}
      PGADMIN_DEFAULT_PASSWORD: ${PGADMIN_DEFAULT_PASSWORD:-admin}
      PGADMIN_CONFIG_SERVER_MODE: 'False'
    ports:
      - "${PGADMIN_PORT:-5050}:80"
mrjones-plip commented 2 weeks ago

@njuguna-n or @witash - before I send this PR out for formal review - can you point me toward how I should add tests for the new bastion container? Maybe an existing pattern we have? I'm a test n00b and need a little guidance :pray:

thanks!

njuguna-n commented 1 week ago

Hello @mrjones-plip for this one you are looking at adding e2e tests right? You can have a look at existing e2e tests here for some guidance.

mrjones-plip commented 5 days ago

While ready for review, I think we should wait until #187 is merged. Then I can get all a green CI on this branch - otherwise right now green CI is impossible because of #186

@lorerod and @dianabarsan - if you can't wait, you can have a preliminary look at e2d and new bastion container respectively! Otherwise, feel free to wait until this PR is fully baked.

mrjones-plip commented 5 days ago

hmmm - I note cleanup never runs, it just hangs here:

 Container cht-sync-postgres-1  Started
      ✔ should handle PostgreSQL downtime gracefully (26937ms)
    Incremental sync
      ✔ should process document edits (18288ms)
      1) should process contact deletes
      ✔ should process person deletes (18039ms)
      ✔ should process report deletes (12023ms)
      ✔ should process incremental inserts (18023ms)
    Conflict resolution
Failed to edit document with id 00172650-a2d1-540b-b308-d53ff75f102c: Document update conflict.
      ✔ should handle update conflicts (19058ms)
Failed to delete document with id 00172650-a2d1-540b-b308-d53ff75f102c: Document update conflict.
      ✔ should handle delete conflicts (19039ms)

  15 passing (3m)
  1 failing

  1) Main workflow Test Suite
       Incremental sync
         should process contact deletes:

      AssertionError: expected 1 to equal +0
      + expected - actual

      -1
      +0

      at Proxy.assertEqual (node_modules/chai/lib/chai/core/assertions.js:1038:12)
      at Proxy.methodWrapper (node_modules/chai/lib/chai/utils/addMethod.js:57:25)
      at doAsserterAsyncAndAddThen (node_modules/chai-as-promised/lib/chai-as-promised.js:289:22)
      at Proxy.<anonymous> (node_modules/chai-as-promised/lib/chai-as-promised.js:255:20)
      at Proxy.overwritingMethodWrapper (node_modules/chai/lib/chai/utils/overwriteMethod.js:78:33)
      at Proxy.<anonymous> (node_modules/chai-exclude/chai-exclude.js:122:21)
      at Proxy.overwritingMethodWrapper (node_modules/chai/lib/chai/utils/overwriteMethod.js:78:33)
      at Context.<anonymous> (file:///home/mrjones/Documents/MedicMobile/cht-sync/tests/e2e-test.spec.js:221:49)
      at process.processTicksAndRejections (node:internal/process/task_queues:105:5)

Maybe I'm not being patient enough? !