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] fix: prevent endless logging from finalize #1082

Merged
merged 2 commits into from
Oct 16, 2024

Conversation

MalinAhlberg
Copy link
Contributor

@MalinAhlberg MalinAhlberg commented Oct 14, 2024

Related issue(s) and PR(s)
This PR closes #1027 .

Description
Added missing case in switch, to make the second call to set-accession finish and not loop infinitely. The fact that the accession id is already set, will later be caught by another check and (hopefully) handled as intended there.

How to test

  1. At the root of the repo, start the stack by running
make build-all
PR_NUMBER=$(/usr/bin/date '+%F')  docker compose -f .github/integration/sda-s3-integration.yml run integration_test
  1. Set the ENVs by
export ACCESS_TOKEN=$(curl -s -k http://localhost:8080/tokens | jq -r '.[0]')
export API_HOST=http://localhost:8090

3 . Ingest a file and set the accession id twice

./sda-admin file ingest --filepath requester_demo.org/data/file1.c4gh --user [email protected]
./sda-admin file set-accession --filepath requester_demo.org/data/file1.c4gh  --user [email protected]   --accession-id my-id1 
./sda-admin file set-accession --filepath requester_demo.org/data/file1.c4gh  --user [email protected]   --accession-id my-id1 
  1. Make sure the logs look ok and that the file's status in the db is good.

@MalinAhlberg MalinAhlberg requested a review from a team October 14, 2024 12:22
@MalinAhlberg MalinAhlberg force-pushed the fix/finalize-logging-double-accession-id branch from 7c5b149 to 31369f7 Compare October 15, 2024 10:02
@MalinAhlberg MalinAhlberg force-pushed the fix/finalize-logging-double-accession-id branch from 31369f7 to 11e197f Compare October 15, 2024 10:05
@MalinAhlberg MalinAhlberg requested a review from a team October 15, 2024 11:23
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.

Great job!

Copy link
Contributor

@kjellp kjellp 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 Oct 16, 2024
Merged via the queue into main with commit d7c7081 Oct 16, 2024
27 checks passed
@jbygdell jbygdell deleted the fix/finalize-logging-double-accession-id branch October 16, 2024 06:48
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.

finalize service logs flood disk with endless warnings after setting accession twice
4 participants