-
Notifications
You must be signed in to change notification settings - Fork 31
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
MM-60769: load from s3 #1629
MM-60769: load from s3 #1629
Conversation
5ec3a17
to
4407652
Compare
utils/packets/aws.py
Outdated
return False | ||
|
||
|
||
# The bucket does not exist or you ha |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
text cut?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left over. Removing.
from utils.packets.service import ingest_support_packet, ingest_survey_packet | ||
from utils.packets.service import ( | ||
ingest_support_packet, | ||
ingest_support_packets_from_s3, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume there is no need to add tests for ingest_support_packets_from_s3
(only the table name changes).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's calling the same function as ingest_surveys_from_s3
. The individual components have been tested. We can add an end-to-end test if you prefer. However, since there might be changes depending on the structure of the S3 bucket, I prefer to wait for them before adding the extra data.
4407652
to
ee2ddf7
Compare
Summary
Assumes that different types of packets are under
<prefix>/YYYY/MM/DD/
.Ticket Link
https://mattermost.atlassian.net/issues/MM-60769