BCDA-7480: Update errors returned from import-cclf #885
Conversation
laurenkrugen-navapbc
requested review from
kyeah,
karinamzalez and
alex-dzeda
bcda/cclf/fileprocessor.go
Outdated
| @@ -119,6 +128,15 @@ func (p *processor) handleArchiveError(path string, info os.FileInfo, cause erro | |||
| return err | |||
| } | |||
|
|
|||
| func isOptOut(filename string) (isOptOut bool) { | |||
There was a problem hiding this comment.
Is this based on an existing function/regex? Just wondering if the CCLF and opt out importers can share the same utility function in case we ever need to change them.
There was a problem hiding this comment.
yeah - I pulled the regex out of this file, but can create an exported util function that both can use! I'll update.
There was a problem hiding this comment.
OK - this has now been moved to the utils.go file used by optout to reduce repetitive code. 🎉
bcda/bcdacli/cli.go
Outdated
| @@ -368,7 +368,7 @@ func setUpApp() *cli.App { | |||
| return err | |||
|
|
|||
| } | |||
| if failure > 0 || skipped > 0 { | |||
| if failure > 0 || skipped > 0 || success == 0 { | |||
There was a problem hiding this comment.
Will this cause an error / alert if no files were available for import?
There was a problem hiding this comment.
Yes, this will cause an alert if no files were successfully imported. There is another piece of logic that will catch an error if we've walked the contents of the directory and have no files that can be imported. I added this because when I was reviewing the logs and the output during the CCLF incident, I did see an instance of 0 files being imported.
There was a problem hiding this comment.
Per our conversation during office hours, this success statement may be a bit too aggressive; removed it!
Codecov Report
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #885 +/- ##
==========================================
+ Coverage 79.20% 79.34% +0.13%
==========================================
Files 93 93
Lines 10600 10611 +11
==========================================
+ Hits 8396 8419 +23
+ Misses 1663 1654 -9
+ Partials 541 538 -3
... and 3 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
🎫 Ticket
https://jira.cms.gov/browse/BCDA-7480
🛠 Changes
Updating
import-cclfcommand to return an error when at least one file has failed to be imported or a file has been skipped, or if the successfully imported file count is 0.Smoke tests are failing because the suppression files get added to the skipped count, if they exist in the directory. Since we don't want false positives, a check has been added to see if the file we are currently importing is an opt out file. If it is, we ignore it altogether and don't increment the count of the files skipped.
ℹ️ Context for reviewers
Smoke tests in dev are failing because opt-out files are being added to the skipped count, which causes the synthetic EFT import to fail.
✅ Acceptance Validation
modified tests that import cclf files to also check for opt out files and not add to skipped count.
🔒 Security Implications
If any security implications apply, add Jason Ashbaugh (GitHub username: StewGoin) as a reviewer and do not merge this PR without his approval.