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

BCDA-7786: Create Job Keys for ndjson files #912

Merged
merged 2 commits into from
Feb 23, 2024

Conversation

laurenkrugen-navapbc
Copy link
Contributor

🎫 Ticket

https://jira.cms.gov/browse/BCDA-7786

🛠 Changes

  • processJob: moved code to create job keys and update job status into its own function for testing
  • processJob: moved file size check into writeBBDataToFile
  • processJob: creating keys for each Job Key that is returned
  • writeBBDataToFile: changed return values to be a list of JobKeys and an error
  • writeBBDataToFile: returning "*-error.ndjson" job key if it is created (was not being returned and did not have job keys previously)
  • update to pre-commit / golangci-lint to check the pkg, as linting was failing on single files.

ℹ️ Context for reviewers

error.ndjson files not having job keys written to the database and middleware checks for job keys failing to serve HTTP. These changes will return the ndjson file and pass the middleware check, if the file exists.

✅ Acceptance Validation

Current tests pass, some tests modified, new test added. Lots of intertwined dependencies with this one; was difficult to test.

🔒 Security Implications

  • This PR adds a new software dependency or dependencies.
  • This PR modifies or invalidates one or more of our security controls.
  • This PR stores or transmits data that was not stored or transmitted before.
  • This PR requires additional review of its security implications for other reasons.

If any security implications apply, add Jason Ashbaugh (GitHub username: StewGoin) as a reviewer and do not merge this PR without his approval.

@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 60.60606% with 13 lines in your changes are missing coverage. Please review.

Project coverage is 79.80%. Comparing base (be7bea0) to head (5f7f20a).

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #912      +/-   ##
==========================================
+ Coverage   79.77%   79.80%   +0.03%     
==========================================
  Files          98       98              
  Lines       10882    10889       +7     
==========================================
+ Hits         8681     8690       +9     
+ Misses       1653     1651       -2     
  Partials      548      548              
Files Coverage Δ
bcdaworker/worker/worker.go 70.38% <60.60%> (+0.32%) ⬆️

... and 2 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update be7bea0...5f7f20a. Read the comment docs.

Copy link
Contributor

@alex-dzeda alex-dzeda left a comment

Choose a reason for hiding this comment

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

Approved; talked about future enhancement to OperationOutcomes for what's written in.

@laurenkrugen-navapbc laurenkrugen-navapbc merged commit 5e6a2cb into master Feb 23, 2024
1 check passed
@laurenkrugen-navapbc laurenkrugen-navapbc deleted the lauren/BCDA-7786-job-keys branch February 23, 2024 18:40
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