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

[ingest] Check for keyhash on startup #1189

Merged
merged 4 commits into from
Dec 13, 2024
Merged

Conversation

jbygdell
Copy link
Collaborator

Related issue(s) and PR(s)

This PR closes #1164 .

Description

Check that a keyhash exists in the database before start processing any messages. A timer counts down for 5 minutes and if no keyhash exists by then an error will be logged and ingest will restart.

No messages will be processed before this check completes.

How to test

make build-all
make sda-s3-up
docker logs -f ingest

"no crypt4gh key hash registered" will be printed every 30 seconds and after 5 minutes the container will restart.

If make integrationtest-sda is used ingest will Print the same message a few times until the tests reaches 11_api_test. Then it will process the messages as normal for the ingestion test.

@jbygdell jbygdell requested a review from a team December 12, 2024 09:58
@jbygdell jbygdell self-assigned this Dec 12, 2024
Copy link
Contributor

@nanjiangshu nanjiangshu left a comment

Choose a reason for hiding this comment

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

Except for some minor comments, the PR works as expected. Great job!

nanjiangshu
nanjiangshu previously approved these changes Dec 12, 2024
Copy link
Contributor

@nanjiangshu nanjiangshu left a comment

Choose a reason for hiding this comment

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

Looks good!

Check that a keyhash exists in the database before start processing any messages. A timer counts down for 5 minutes and if no keyhash exists by then an error will be logged and ingest will restart.
@jbygdell
Copy link
Collaborator Author

Rebased so needs to be reapproved.

@jbygdell jbygdell enabled auto-merge December 13, 2024 08:37
Copy link
Contributor

@aaperis aaperis left a comment

Choose a reason for hiding this comment

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

LGTM

@jbygdell jbygdell added this pull request to the merge queue Dec 13, 2024
Merged via the queue into main with commit aab3e14 Dec 13, 2024
28 checks passed
@jbygdell jbygdell deleted the feature/ingest-check-for-hash branch December 13, 2024 10:30
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.

ingest logs fills up memory with error logs
3 participants