Skip to content

[18.0][IMP] dms: Clean up attachments after creation of file#470

Open
tarteo wants to merge 1 commit intoOCA:18.0from
tarteo:18-fix-gc-attachment
Open

[18.0][IMP] dms: Clean up attachments after creation of file#470
tarteo wants to merge 1 commit intoOCA:18.0from
tarteo:18-fix-gc-attachment

Conversation

@tarteo
Copy link
Copy Markdown
Member

@tarteo tarteo commented Mar 27, 2026

Fixes: #468

@pedrobaeza pedrobaeza added this to the 18.0 milestone Mar 27, 2026
Copy link
Copy Markdown
Member

@pedrobaeza pedrobaeza left a comment

Choose a reason for hiding this comment

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

Why an attachment is created in first place?

@tarteo
Copy link
Copy Markdown
Member Author

tarteo commented Apr 7, 2026

@pedrobaeza It reuses /web/binary/upload_attachment that creates the attachment, but functionally I don't think there's a reason.

@pedrobaeza
Copy link
Copy Markdown
Member

Then isn't better to do the upload other way for not generating that garbage?

@tarteo
Copy link
Copy Markdown
Member Author

tarteo commented Apr 7, 2026

@pedrobaeza I don't know if it's used for something, I assume there is / was a reason to use the endpoint.

@pedrobaeza
Copy link
Copy Markdown
Member

I don't think so apart from reducing the code in this part, but having that side effects, this is to be questioned. @victoralmau can you confirm?

@victoralmau
Copy link
Copy Markdown
Member

I don't think so apart from reducing the code in this part, but having that side effects, this is to be questioned. @victoralmau can you confirm?

I see that this was added in v16 (#262), since there is already a method in the web version with similar functionality (https:// github.com/odoo/odoo/blob/af32885ec5f07d492f3b8e8fff1785996a739f72/addons/web/controllers/binary.py#L217), I infer that the intention was to reduce the code a bit and use it; therefore, the change added in this PR is necessary.

@pedrobaeza
Copy link
Copy Markdown
Member

Yes, but as said, with this high side effect, I would switch again to have the complete upload handle here in this module instead of using the other, but generating the garbage. Maybe a part of the other code can be used without generating the attachment.

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.

[18.0] dms: attachment created during uploading of a new file is new garbage collected

3 participants