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

Added submissions/latest/report and submissions/{id}/report, added pytest #144

Merged

Conversation

jcadam14
Copy link
Contributor

@jcadam14 jcadam14 commented Apr 5, 2024

Closes #138

  • Added endpoints /submissions/latest/report and /submissions/{id}/report
  • Added get_from_storage function in submission_processor
  • Added pytests
  • Small update include period_code to validation function in submission_processor so we can store results
  • Added call to df_to_download after getting validation results

@jcadam14 jcadam14 self-assigned this Apr 5, 2024
@jcadam14 jcadam14 linked an issue Apr 5, 2024 that may be closed by this pull request
Copy link

github-actions bot commented Apr 5, 2024

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  src/sbl_filing_api/entities/repos
  submission_repo.py
  src/sbl_filing_api/routers
  filing.py
  src/sbl_filing_api/services
  submission_processor.py
Project Total  

This report was generated by python-coverage-comment-action

@jcadam14 jcadam14 requested a review from lchen-2101 April 8, 2024 22:00
Copy link
Member

@hkeeler hkeeler 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. Just the one question.

I'm going to go ahead and mark it Approved. If you end up making the switch to FastAPI's FIleRepsonse here, I can rereview when ready.

Copy link
Member

Choose a reason for hiding this comment

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

Is this file intended to change for this PR? Is this just the result of pulling the latest from https://github.com/cfpb/regtech-data-validator.git and some other changes came along for the ride? I'm a little surprised content-hash at the end of the file didn't change along with this. Maybe that doesn't change when the dependency is just referencing a git repo?

Poetry continues to make me go 🤔 .

Copy link
Collaborator

@lchen-2101 lchen-2101 left a comment

Choose a reason for hiding this comment

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

LGTM

@lchen-2101 lchen-2101 merged commit f137e77 into main Apr 9, 2024
3 checks passed
@lchen-2101 lchen-2101 deleted the 138-add-endpoint-for-downloading-csv-of-validation-results branch April 9, 2024 14:07
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.

Add endpoint for downloading CSV of validation results
3 participants