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

Add docker setup for bulk export #41

Merged
merged 15 commits into from
Apr 15, 2024
Merged

Add docker setup for bulk export #41

merged 15 commits into from
Apr 15, 2024

Conversation

elsaperelli
Copy link
Collaborator

Summary

This PR sets up a working Docker environment for this server and changes some things so that it can be run on our abacus-dev ECE.

New behavior

Server can be run with Docker! Also pushed this repository to docker hub and added it to abacus-dev.

Code changes

  • .env - adds PUBLIC_BULK_SERVER variable and changes PORT to 3000 for consistency
  • .env.test - adds PUBLIC_BULK_SERVER variable
  • Dockerfile - upgrades node, adds ENV HOST and ENV PORT variables
  • docker-compose.example.yml - example docker-compose for future reference (similar to that in the measure-repository)
  • docker-compose.yml - add redis, upgrade mongo, add PUBLIC_BULK_SERVER
  • postTransactionBundles.js/bulkstatus.service.js/export.service.js/bulkstatus.service.test.js - use PUBLIC_BULK_SERVER

Testing guidance

  • npm run check
  • Start up the server with docker with docker compose up --build
  • Use Insomnia to ensure functionality is the same as if the server was spun up locally. See PR Add support for the _elements param #39 for example Insomnia requests to get started.
  • Start the server locally to ensure that all runs the same.

@elsaperelli elsaperelli marked this pull request as ready for review April 10, 2024 16:54
Copy link

github-actions bot commented Apr 10, 2024

Coverage report

St.
Category Percentage Covered / Total
🟡 Statements
79.59% (-0.96% 🔻)
468/588
🟡 Branches
66.53% (-1.08% 🔻)
167/251
🟡 Functions 77.89% 74/95
🟢 Lines
80.03% (-0.84% 🔻)
461/576
Show files with reduced coverage 🔻
St.
File Statements Branches Functions Lines
🔴
... / bundle.service.js
13.85% (-1.67% 🔻)
0% 0%
14.06% (-1.45% 🔻)

Test suite run success

76 tests passing in 7 suites.

Report generated by 🧪jest coverage report action from ab5001f

Copy link
Contributor

@lmd59 lmd59 left a comment

Choose a reason for hiding this comment

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

Testing through things a bit more, but a couple of small initial suggestions

Dockerfile Outdated Show resolved Hide resolved
docker-compose.example.yml Outdated Show resolved Hide resolved
Copy link
Contributor

@hossenlopp hossenlopp left a comment

Choose a reason for hiding this comment

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

Some of the stuff in the docker-compose can be paired down and moved to the Dockerfile.

Suggest renaming PUBLIC_BULK_SERVER to something that implies it is the base URL. Perhaps BULK_BASE_URL.

We could also use a script for running and pushing the build like we have in measure-repository service.

docker-compose.yml Outdated Show resolved Hide resolved
docker-compose.yml Outdated Show resolved Hide resolved
docker-compose.yml Outdated Show resolved Hide resolved
docker-compose.example.yml Outdated Show resolved Hide resolved
docker-compose.example.yml Outdated Show resolved Hide resolved
docker-compose.yml Outdated Show resolved Hide resolved
@elsaperelli
Copy link
Collaborator Author

@hossenlopp I first misread your comment thinking you asked for a bundle upload script...but now we have both! Added a directory-upload.sh script and a docker-build.sh script like we do in the measure-repository.

@elsaperelli elsaperelli requested review from lmd59 and hossenlopp April 12, 2024 14:06
Copy link
Contributor

@lmd59 lmd59 left a comment

Choose a reason for hiding this comment

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

One small comment/question. Otherwise I think it looks good!

Dockerfile Outdated Show resolved Hide resolved
@elsaperelli elsaperelli requested a review from hossenlopp April 15, 2024 14:56
@elsaperelli elsaperelli requested a review from lmd59 April 15, 2024 14:56
@elsaperelli elsaperelli merged commit 4c00e28 into main Apr 15, 2024
4 checks passed
@elsaperelli elsaperelli deleted the fix-docker branch April 15, 2024 15:59
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