Skip to content

Database Dumps to S3#810

Merged
BrendannnD merged 4 commits intomasterfrom
S3Upload
Mar 5, 2026
Merged

Database Dumps to S3#810
BrendannnD merged 4 commits intomasterfrom
S3Upload

Conversation

@lab596
Copy link
Copy Markdown
Contributor

@lab596 lab596 commented Feb 24, 2026

This pull request introduces a new mechanism for database backup in the DbDumpService, enabling automated database dumps and uploads to an S3-compatible endpoint, with robust fallback and configuration logic. It also adds comprehensive unit and integration tests to ensure the reliability of the new backup workflow.

Key changes:

Database Dump and S3 Upload Enhancements

  • Implemented logic in DbDumpService to create a database dump, compress it, and upload it to an S3 endpoint if configured. If the S3 endpoint is unavailable, the service falls back to storing the dump locally. The S3 endpoint and kit ID are now dynamically read from environment variables, and the upload process includes detailed logging and error handling.
  • Added utility methods to determine the kit ID (from KIT_ID, TRIP_ID, hostname, or a default), and to handle HTTP response reading and error reporting for the S3 upload.

Testing and Validation

  • Added DbDumpServiceTest to provide integration tests for the S3 backup process, including tests for database dump creation, S3 endpoint configuration, and kit ID detection. These tests use environment variables and conditional logic to adapt to different test environments.
  • Introduced DbDumpServiceMockTest with unit tests to validate configuration reading, file path construction, endpoint URL formatting, timeout configuration, fallback behavior, and kit ID priority logic, all without requiring a live database or AWS account.


ServiceResponse<Boolean> serviceResponse = new ServiceResponse<>();
// Use absolute path in /tmp to ensure file is created in a known location
String dumpFilePath = "/tmp/db_dump.sql.gz";
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

S3 upload needs an absolute filepath

("mysqldump", "--host=db", String.format("--user=%s", db_user),
String.format("--password=%s", db_password), "--all-databases");

File outputFile = new File(dumpFilePath);
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

here we are writing the dump file so we can acess using the dumpFilePath

Logger.info("DbDumpService", "Database dump created: " + dumpFilePath);

// Step 2: Upload to S3 if endpoint is configured
String s3Endpoint = System.getenv("S3_BACKUP_ENDPOINT"); // Read dynamically
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is currently hidden in the docket compose until auth is taken care of

*
* @return kit ID string
*/
private String getKitId() {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

reason for this is to add this to the name of the S3 dump so we can identify it

import femr.data.daos.core.IEncounterRepository;
import femr.data.daos.core.IPatientRepository;
import femr.data.daos.core.IPrescriptionRepository;
import femr.data.models.core.IConceptDiagnosis;
Copy link
Copy Markdown
Contributor Author

@lab596 lab596 Mar 3, 2026

Choose a reason for hiding this comment

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

unrelated failing test fix

@lab596
Copy link
Copy Markdown
Contributor Author

lab596 commented Mar 3, 2026

Issues Identified from code review:

image
  1. Cannot do this since you need to be admin to the repo - can ask BJ to add after merge
  2. It is possible that 30s is too short but is hard to test and verify here, dont want to arbitrarily increase since it is hard to settle on a hardcoded value like this
  3. All services actually use a addError format to throw errors and custom exceptions that are thrown the frontend are actually swallowed so throwing custom exceptions here will not provide any additional benefit, good call out though

@BrendannnD BrendannnD closed this Mar 5, 2026
@BrendannnD BrendannnD reopened this Mar 5, 2026
public class DbDumpService implements IDbDumpService {

// S3 upload timeout configuration
private static final int S3_UPLOAD_TIMEOUT = 30000; // 30 seconds
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

With very crappy computers I'm not sure if this timeout is too restrictive.

@BrendannnD BrendannnD merged commit 8b6f6b9 into master Mar 5, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants