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 upload file funcitonality #22

Merged
merged 9 commits into from
Feb 27, 2024
Merged

Added upload file funcitonality #22

merged 9 commits into from
Feb 27, 2024

Conversation

nargis-sultani
Copy link
Contributor

@nargis-sultani nargis-sultani commented Jan 10, 2024

closes #4

@nargis-sultani
Copy link
Contributor Author

The purpose of this ticket was to research and get familiarized with uploading to S3. This PR includes the function to upload to s3, but it is not directly linked to S3. I am using "moto" library to mock s3 for testing.
Currently, I am using "lei" for bucket name and submission_id for object key. I can discuss with @lchen-2101 on how the file layout should be on s3.

@nargis-sultani nargis-sultani changed the title Added function to upload to s3, but currently without credential support Added function to upload to s3, but currently without directly linking to s3 Jan 10, 2024
@lchen-2101 lchen-2101 marked this pull request as draft February 15, 2024 15:56
Copy link

github-actions bot commented Feb 26, 2024

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  src
  config.py
  src/routers
  filing.py 48
  src/services
  submission_processor.py
Project Total  

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

@lchen-2101 lchen-2101 changed the title Added function to upload to s3, but currently without directly linking to s3 Added upload file funcitonality Feb 26, 2024
@lchen-2101 lchen-2101 marked this pull request as ready for review February 26, 2024 20:02
@lchen-2101 lchen-2101 requested a review from jcadam14 February 26, 2024 21:12
src/config.py Outdated Show resolved Hide resolved
f.write(content)
except Exception as e:
log.error("Failed to upload file", e, exc_info=True, stack_info=True)
raise HTTPException(status_code=HTTPStatus.INTERNAL_SERVER_ERROR, detail="Failed to upload file")
Copy link
Contributor

Choose a reason for hiding this comment

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

If the SubmissionDAO is getting created prior to this possible exception, what would happen to that object? Should we remove it from the database, or set a state on it? Originally the SUBMISSION_UPLOADED was going to be set after saving to S3, but since the object will be created before it now, would it just stay in that state and from that we can infer something bad happened (because it didn't move to VALIDATION_IN_PROGRESS)? This is for #52 really, but wanted to get thoughts so that story is implemented correctly.

Copy link
Collaborator

Choose a reason for hiding this comment

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

we'll be setting a state on it; so it's an additional one of upload failed, which I don't think we have yet.

Copy link
Contributor

@jcadam14 jcadam14 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

@jcadam14 jcadam14 merged commit 0b09821 into main Feb 27, 2024
3 checks passed
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.

File upload through FastAPI
3 participants