feat(#12): cloud-based execution of move contact #177

paulpascal · 2024-05-18T02:03:03Z

Closes: #12

This PR introduces a cloud-based execution of the move-contact feature, enhancing user experience by replacing the previous cht-conf command-line approach.

To achieve this, we've implemented bullmq, a powerful and flexible queue system built on Redis.

Here's a breakdown of the process:

Upon initiating a contact move, essential details such as contact_id, parent_id, instanceUrl, and the current sessionToken (encrypted) are compiled and submitted as a job to the MOVE_CONTACT_QUEUE.
A dedicated worker monitors this queue, processing incoming jobs:
- It has the flexibility to defer processing based on predefined logic, allowing for scheduled execution.
- Alternatively, it can proceed to execute the cht-conf command via a child_process, utilizing all pertinent job data, including the decrypted sessionToken.
Upon successful execution, the job is marked as completed. In case of any failures, the job is flagged accordingly. Users can conveniently track job statuses directly from the user management tool's dashboard.

Note

Enabling cht-conf to support session tokens required implementing some modifications, detailed here.
(the session token is initial got from _session request)

…ve_contact

…move_contact

mrjones-plip

Really good work here - great to see a compose file that I suspect works out of the box!

My big question is how we're building and publishing images. I have some smaller feedback points and tried to make code ```suggestions when possible - but lemme know if I got any of those wrong!

docker-compose.yml

paulpascal · 2024-05-21T09:24:03Z

Thanks @mrjones-plip for this great review, I will be going through them one by one and get back to you.
About the publishing, I was also thinking, should it be possible to do it the same way we did for the current version (using publish script). Maybe it will need some updates on the current docker stuff I did? (Forgive if I'm sinning here 🥲 I am not a 😅pro on docker)

cc: @kennsippell

kennsippell

Amazing work Paul!

Dockerfile

package.json

shared/config/index.ts

shared/encryption/index.ts

worker/worker.ts

worker/jobs/move-contact/move-contact-job.processor.ts

worker/worker.ts

paulpascal · 2024-05-21T17:08:16Z

Thanks @kennsippell , great review. Really appreciate. Will be making the right adjustment

kennsippell · 2024-05-21T23:32:33Z

Testing today:

If redis service is not running, the worker process logs errors but doesn't exit. Seems it should fail fast.
The Cht command output: output with all the new lines is extremely verbose. I think we can just output the raw data. It'd also be nice to output the actual command being executed.'
Can we add json_docs and upload-docs.*.json into .gitignore since this is created when worker thread is run in active directory.
We need to think about how to handle the case where the docker container is killed (eg. deployment) while a job is executing. This can leave contacts in a broken state.
adding new job to MOVE_CONTACT_QUEUE with [object Object]

worker/jobs/move-contact/move-contact-job.processor.ts

…ove-contact-cloud-based

kennsippell

Great stuff Paul. I did another review here but I got tired and didn't look through tests yet.

src/worker/move-contact-worker.ts

src/shared/queue-config.ts

src/worker/move-contact-worker.ts

test/worker/move-contact-worker.spec.ts

src/worker/move-contact-worker.ts

paulpascal · 2024-06-10T13:17:35Z

Hi, @mrjones-plip it took a bit but finally got all the docker feedback addressed 🥲. Would you like to check it back when ever you have time?

@kennsippell, i have fixed the delay stuff as well.

Thanks

kennsippell

Very very nice progress. This feels close.

README.md

package.json

src/lib/authentication.ts

src/worker/move-contact-worker.ts

test/lib/authentication.spec.ts

test/lib/move.spec.ts

test/worker/move-contact-worker.spec.ts

kennsippell · 2024-06-11T23:08:29Z

src/worker/move-contact-worker.ts

+      job.log(`[${new Date().toISOString()}]: ${result.message}`);
+      const errorMessage = `Job ${job.id} failed with the following error: ${result.message}`;
+      console.error(errorMessage);
+      throw new Error(errorMessage);


at some point it might be nice to handle errors a bit better. maybe not in this PR, but what if cht-conf has an error - the most common scenario here is losing connectivity with the server during a heavy move. should the job be retried?

i'll make a quick comment about aborted/canceled jobs or generally handling any case where a contact's move is interrupted

the good news is that if a move is interrupted, regardless of the state cht-conf can be rerun. it will aggregate all documents (those in the new position already and those in the old position) and move them all again. this is really an incredible win - if a job fails, it just needs to be retried.

the bad news is that cht-user-management won't let you retry an aborted job if the top-level contact successfully moves. it will give an error like "A already has B as parent" or whatever.

It's going to happen, and we're going to need to deal with it at some point.

This is really important, the way I was dealing with it is to see from the Board, the issue with the failed job (from the logs).
If it has failed due to the lost of connectivity, then I just retry from the Board.

But we can handle this properly in another PR.

can you make an issue so we don't forget?

mrjones-plip

yes! Looking really good here. Most of my feedback is trying both make it easy to set up the dev environment, while also keeping sane defaults for production.

While I did easily stand up an instance using the bash script, I'm unable to connect to redis for some reason.

both the main service and the work service have this error, where 192.168.16.2 is the IP of the redis container:

docker logs cht-user-management-cht-user-management-1

Error: connect ECONNREFUSED 192.168.16.2:6378
    at TCPConnectWrap.afterConnect [as oncomplete] (node:net:1606:16) {
  errno: -111,
  code: 'ECONNREFUSED',
  syscall: 'connect',
  address: '192.168.16.2',
  port: 6378
}

does this work for you?

README.md

env.example

docker-local-setup.sh

Dockerfile.worker

Dockerfile

README.md

Dockerfile.worker

env.example

paulpascal · 2024-06-12T16:00:58Z

Thanks @mrjones-plip and @kennsippell for your great reviews 🔥.

Really appreciate that, will go through it thoroughly.

Co-authored-by: Ashley <8253488+mrjones-plip@users.noreply.github.com> Co-authored-by: Kenn Sippell <kennsippell@gmail.com>

paulpascal · 2024-06-19T16:36:48Z

docker logs cht-user-management-cht-user-management-1

Error: connect ECONNREFUSED 192.168.16.2:6378
    at TCPConnectWrap.afterConnect [as oncomplete] (node:net:1606:16) {
  errno: -111,
  code: 'ECONNREFUSED',
  syscall: 'connect',
  address: '192.168.16.2',
  port: 6378
}

does this work for you?

Yes @mrjones-plip, it works for me, we can please have a little sync on that if its okay with you.

…ove-contact-cloud-based

kennsippell

Let's do one more, but I think we're basically there.

README.md

src/lib/authentication.ts

src/lib/queues.ts

src/lib/move.ts

kennsippell · 2024-06-24T17:56:32Z

src/worker/move-contact-worker.ts

+      job.log(`[${new Date().toISOString()}]: ${result.message}`);
+      const errorMessage = `Job ${job.id} failed with the following error: ${result.message}`;
+      console.error(errorMessage);
+      throw new Error(errorMessage);


can you make an issue so we don't forget?

src/worker/move-contact-worker.ts

src/lib/authentication.ts

test/worker/move-contact-worker.spec.ts

mrjones-plip

Nice work @paulpascal! I REALLY love that NODE_ENV is exposed in the .env file now so it makes it trivial to run in dev mode in docker - so handy!

I'm not getting any more errors for redis service in the docker log, so I assume this is working? I don't know how to test.

I did have one issue that I wasn't able log in - here's what I did:

deploy 4.8.1 instance with docker helper 192-168-68-26.local-ip.medicmobile.org:10446
set CHT_DEV_HTTP=true and CHT_DEV_URL_PORT=192-168-68-26.local-ip.medicmobile.org:10446 and NODE_ENV=dev
in CHT Core, create a test user that is role User Manager
start user man tool with: ./docker-local-setup.sh build
try to login with that user on 127.0.0.1:3000/login

expected: I can log in
actual: get error "login failed. please try again"

Here's my .env in case it's helpful:

NODE_ENV=dev
COOKIE_PRIVATE_KEY=quux
QUEUE_PRIVATE_KEY=smang
CONFIG_NAME=chis-ke
PORT=3000
EXTERNAL_PORT=3000
INTERFACE=0.0.0.0
CHT_DEV_HTTP=true
CHT_DEV_URL_PORT=192-168-68-26.local-ip.medicmobile.org:10446

docker-local-setup.sh

env.example

README.md

env.example

Dockerfile

Co-authored-by: mrjones <8253488+mrjones-plip@users.noreply.github.com> Co-authored-by: Kenn Sippell <kennsippell@gmail.com>

mrjones-plip

From a docker and docker compose perspective, with the updated env.example and updated readme.md - I think this PR is ready to go from my perspective. I still was unable to log in, so please be sure and test this with a clean set up to ensure there's no regressions!

paulpascal · 2024-06-26T20:54:55Z

... I still was unable to log in, so please be sure and test this with a clean set up to ensure there's no regressions!

Thanks @mrjones-plip , I will make sure to have that working.

kennsippell

Amazing! Let's go!

src/server.ts

test/scripts/wait_for_redis.sh

paulpascal and others added 6 commits May 15, 2024 14:13

wip: move contact setup raw feature

04acc66

Merge branch main of github.com:medic/cht-user-management into wip_mo…

2aefcb0

…ve_contact

feat(#12): add centralized queue layer & cleanup

60f0146

Merge branch 'main' of github.com:medic/cht-user-management into wip_…

5ab5aaa

…move_contact

fix(#12): oups test

dff28a7

fix(#12): get this test pass

658f9f8

paulpascal mentioned this pull request May 18, 2024

Add support for AuthSession cookie medic/cht-conf#617

Merged

paulpascal force-pushed the 12-move-contact-cloud-based branch from d28373b to d8eb298 Compare May 20, 2024 13:30

fix(#12): lint

d770b31

paulpascal force-pushed the 12-move-contact-cloud-based branch from d8eb298 to d770b31 Compare May 20, 2024 13:32

mrjones-plip requested changes May 21, 2024

View reviewed changes

kennsippell requested changes May 21, 2024

View reviewed changes

kennsippell mentioned this pull request May 22, 2024

Script to bulk move contacts from CSV #178

Draft

kennsippell reviewed May 23, 2024

View reviewed changes

worker/jobs/move-contact/move-contact-job.processor.ts Outdated Show resolved Hide resolved

paulpascal added 3 commits May 30, 2024 17:57

feat: address feedback part 1

4fd502c

fix(#12): failing test

2bd987b

Merge branch 'main' of github.com:medic/cht-user-management into 12-m…

b704535

…ove-contact-cloud-based

kennsippell requested changes Jun 3, 2024

View reviewed changes

kennsippell reviewed Jun 3, 2024

View reviewed changes

src/worker/move-contact-worker.ts Outdated Show resolved Hide resolved

paulpascal added 7 commits June 4, 2024 04:00

feat(#12): fix review feedback part2

ac90d8e

feat(#12): fix review feedback part2

6a47307

fix(#12): job postponed

f83d358

fix(#12): update postponed test

7aad96e

feat(#12): make this toast display

62b2a9d

ref(#12): remove move_result.html as no more used

a5f93eb

fix(#12): address docker feedback

189c6a8

paulpascal requested a review from kennsippell June 10, 2024 09:53

paulpascal requested a review from mrjones-plip June 10, 2024 09:53

kennsippell requested changes Jun 11, 2024

View reviewed changes

mrjones-plip requested changes Jun 11, 2024

View reviewed changes

Apply suggestions from code review

feffcf1

Co-authored-by: Ashley <8253488+mrjones-plip@users.noreply.github.com> Co-authored-by: Kenn Sippell <kennsippell@gmail.com>

paulpascal force-pushed the 12-move-contact-cloud-based branch from d88902a to c5cc101 Compare June 19, 2024 16:34

paulpascal requested review from mrjones-plip and kennsippell June 19, 2024 18:09

paulpascal added 3 commits June 20, 2024 16:34

fix(#12): address feedback part 5

c5cc101

Merge branch 'main' of github.com:medic/cht-user-management into 12-m…

6f48e8b

…ove-contact-cloud-based

ref(#12): switch to an official version of cht-conf support session

31e91d1

paulpascal force-pushed the 12-move-contact-cloud-based branch from 594f62f to 31e91d1 Compare June 22, 2024 02:57

kennsippell requested changes Jun 24, 2024

View reviewed changes

mrjones-plip requested changes Jun 24, 2024

View reviewed changes

docker-local-setup.sh Show resolved Hide resolved

env.example Outdated Show resolved Hide resolved

README.md Outdated Show resolved Hide resolved

env.example Show resolved Hide resolved

Dockerfile Outdated Show resolved Hide resolved

paulpascal and others added 2 commits June 25, 2024 01:41

Apply suggestions from code review

87a8af1

Co-authored-by: mrjones <8253488+mrjones-plip@users.noreply.github.com> Co-authored-by: Kenn Sippell <kennsippell@gmail.com>

doc(#12): update readme

0ea2be0

mrjones-plip approved these changes Jun 26, 2024

View reviewed changes

paulpascal force-pushed the 12-move-contact-cloud-based branch from 7b512fe to ef2573a Compare June 27, 2024 01:29

paulpascal mentioned this pull request Jun 27, 2024

Move Contact: Improve Error Handling and Retry Logic for Interrupted/Failed #184

Open

paulpascal requested a review from kennsippell June 27, 2024 01:40

fix(#12): add integration test + feedback

cf89625

paulpascal force-pushed the 12-move-contact-cloud-based branch from ef2573a to cf89625 Compare June 27, 2024 02:37

kennsippell approved these changes Jun 28, 2024

View reviewed changes

src/server.ts Show resolved Hide resolved

test/scripts/wait_for_redis.sh Outdated Show resolved Hide resolved

1.4.0

8ee35c1

paulpascal merged commit 975d57c into main Jun 29, 2024
1 check passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(#12): cloud-based execution of move contact #177

feat(#12): cloud-based execution of move contact #177

paulpascal commented May 18, 2024 •

edited

Loading

mrjones-plip left a comment

paulpascal commented May 21, 2024

kennsippell left a comment

paulpascal commented May 21, 2024

kennsippell commented May 21, 2024 •

edited

Loading

kennsippell left a comment

paulpascal commented Jun 10, 2024

kennsippell left a comment

kennsippell Jun 11, 2024

paulpascal Jun 20, 2024

kennsippell Jun 24, 2024

mrjones-plip left a comment

paulpascal commented Jun 12, 2024

paulpascal commented Jun 19, 2024 •

edited

Loading

kennsippell left a comment

kennsippell Jun 24, 2024

mrjones-plip left a comment

mrjones-plip left a comment

paulpascal commented Jun 26, 2024

kennsippell left a comment

feat(#12): cloud-based execution of move contact #177

feat(#12): cloud-based execution of move contact #177

Conversation

paulpascal commented May 18, 2024 • edited Loading

mrjones-plip left a comment

Choose a reason for hiding this comment

paulpascal commented May 21, 2024

kennsippell left a comment

Choose a reason for hiding this comment

paulpascal commented May 21, 2024

kennsippell commented May 21, 2024 • edited Loading

kennsippell left a comment

Choose a reason for hiding this comment

paulpascal commented Jun 10, 2024

kennsippell left a comment

Choose a reason for hiding this comment

kennsippell Jun 11, 2024

Choose a reason for hiding this comment

paulpascal Jun 20, 2024

Choose a reason for hiding this comment

kennsippell Jun 24, 2024

Choose a reason for hiding this comment

mrjones-plip left a comment

Choose a reason for hiding this comment

paulpascal commented Jun 12, 2024

paulpascal commented Jun 19, 2024 • edited Loading

kennsippell left a comment

Choose a reason for hiding this comment

kennsippell Jun 24, 2024

Choose a reason for hiding this comment

mrjones-plip left a comment

Choose a reason for hiding this comment

mrjones-plip left a comment

Choose a reason for hiding this comment

paulpascal commented Jun 26, 2024

kennsippell left a comment

Choose a reason for hiding this comment

paulpascal commented May 18, 2024 •

edited

Loading

kennsippell commented May 21, 2024 •

edited

Loading

paulpascal commented Jun 19, 2024 •

edited

Loading