-
-
Notifications
You must be signed in to change notification settings - Fork 213
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
reference form attachments in case attachments instead of copying #13200
Conversation
@run_with_all_backends | ||
def test_case_attachment(self): | ||
single_attach = 'fruity_file' | ||
xform, case = self._doCreateCaseWithMultimedia(attachments=[single_attach]) |
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.
This sends attachments to submit_form_locally
, which I would expect to work. I thought case attachments were working, but still stored in couch. Is that not true?
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.
Case attachments come from form attachments hence the call to
submit_form_locally
commcare-hq/corehq/ex-submodules/casexml/apps/case/tests/test_multimedia.py
Lines 94 to 100 in 9a9de02
response, form, cases = submit_form_locally( | |
xml_data, | |
TEST_DOMAIN_NAME, | |
attachments=dict_attachments, | |
last_sync_token=sync_token, | |
received_on=date | |
) |
The SQL backend uses the blobdb for all attachments (form and case).
Currently we duplicate form attachments for cases so one thing we could
consider is just referencing the same blob in both the form and case.
I also see that boto S3 has a copy
API method which we could also use
rather than doing a read and then write.
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'm trying out the copy API right now. Will have a PR soon, hopefully.
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.
9a9de02
to
ec2781f
Compare
ec2781f
to
7c9b7ff
Compare
7c9b7ff
to
7d039ed
Compare
with trap_extra_setup(AttributeError, msg="S3_BLOB_DB_SETTINGS not configured"): | ||
config = settings.S3_BLOB_DB_SETTINGS | ||
self.s3db = TemporaryS3BlobDB(config) | ||
assert get_blob_db() is self.s3db, (get_blob_db(), self.s3db) |
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.
nit: the above two lines don't need to (and probably shouldn't) be in the with trap_extra_setup
block.
👍 on ✅ |
✅ |
@millerdev
buddy @calellowitz