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

Epic/attribution lambda generalization #1022

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

carlpartridge
Copy link
Collaborator

🎫 Ticket

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

🛠 Changes

Convert file attribution lambda to be more generalized by allowing for CSV files to be ingested as well.

ℹ️ Context

This is a large epic including multiple feature branches. Overall the goal is to convert the file attribution lambda to be more generalized by allowing for CSV files to be ingested as well.

🧪 Validation

Each individual feature branch was PRd before merging into epic branch. Each feature branch was also passing local and git linting and testing.

austincanada and others added 3 commits November 8, 2024 09:02
## 🎫 Ticket

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

## 🛠 Changes

<!-- What was added, updated, or removed in this PR? -->
Added the main entry to the CCLF lambda.
Added the logic flow for checking if the SQS event contains a CSV file
path, or CCLF.
Renamed CCLF lambda functions to attribution agnostic.
Added parser functions for checking CSV file.
Added testing for CSV parser functions.

## ℹ️ Context

These changes will be the entry for the lambda changes in order to make
the CCLF lambda more agonistic to an attribution lambda.

## 🧪 Validation

This PR is a part of a larger group of PR's that will come later. Some
of these entries don't function properly, such as the CSVImporter.
Therefore this PR is more-or less a check for making sure the
functionality is up to standards for the team.
## 🎫 Ticket

https://jira.cms.gov/browse/BCDA-4443
https://jira.cms.gov/browse/BCDA-4444

## 🛠 Changes

- CSV file import logic has been created that works with local file
handler and s3 file handler
- tests added/updated
- removed most logging statements so that errors are pushed to
`handlecsvimport`, where there is logging already. this reduces log
noise
- added custom errors to improve readability and error handling

## ℹ️ Context

Changes were made to support attribution files in a csv file format.
Follows the flow and structure of the CCLF file importer and was adapted
to fit expected csv file format.

<!-- If any of the following security implications apply, this PR must
not be merged without Stephen Walter's approval. Explain in this section
and add @SJWalter11 as a reviewer.
  - Adds a new software dependency or dependencies.
  - Modifies or invalidates one or more of our security controls.
  - Stores or transmits data that was not stored or transmitted before.
- Requires additional review of security implications for other reasons.
-->

## 🧪 Validation

Added new tests and current tests passing.

---------

Co-authored-by: Parwinder Bhagat <Parwinder.Bhagat@e14s.com>
## 🎫 Ticket

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

## 🛠 Changes

- github workflow for attribution integration test has been updated to
include the csv file type

## ℹ️ Context

Made to support csv attribution types for models that utilize csv
attribution files.

<!-- If any of the following security implications apply, this PR must
not be merged without Stephen Walter's approval. Explain in this section
and add @SJWalter11 as a reviewer.
  - Adds a new software dependency or dependencies.
  - Modifies or invalidates one or more of our security controls.
  - Stores or transmits data that was not stored or transmitted before.
- Requires additional review of security implications for other reasons.
-->

## 🧪 Validation

Integration test github action has been updated to include steps to
generate a csv attribution file and ingest it. Test is passing and
manually validated that database entries looks OK.
bcda/cclf/csv.go Outdated Show resolved Hide resolved
bcda/cclf/csv.go Outdated Show resolved Hide resolved
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.

4 participants