feat(attachment): Inline small attachments instead of uploading to objectstore#6165
Conversation
| #[serde( | ||
| rename = "relay.attachment-inline.limit", | ||
| deserialize_with = "default_on_error", | ||
| skip_serializing_if = "is_default" | ||
| )] | ||
| pub attachment_inline_limit: u32, | ||
|
|
There was a problem hiding this comment.
Idea here is that we can roll this change out as is (NOOP) and once we set this value it would take effect. Would also allow us to experiment with different values.
There was a problem hiding this comment.
Talked with Joris and David offline and making this a usize should be fine.
| match upload_context.upload_decision(item.attachment_type()) { | ||
| UploadDecision::Inline => read_inline(field, item).await, | ||
| UploadDecision::Upload => { | ||
| let content_type = field.content_type().map(|ct| ct.essence_str().to_owned()); |
There was a problem hiding this comment.
I think we want to use essence_str in general (we currently do it differently at a couple of spots in the new logi so I think we want to fix that in a follow up).
There was a problem hiding this comment.
Plan: Do it like the rest of the code (ct.as_ref()) and as part of INGEST-983 clean this up overall.
| /// The result of [`split_by_size`]. | ||
| pub enum SizeSplit<S> { | ||
| /// The stream is less than limit and hence buffered into memory. | ||
| Small(Bytes), | ||
| /// The stream exceeds limit and hence is not buffered. | ||
| Large(S), | ||
| } | ||
|
|
||
| /// Buffers a stream if its size is less than `limit` bytes, else return the stream. | ||
| pub async fn split_by_size<S, E>( | ||
| stream: S, | ||
| limit: usize, | ||
| ) -> Result<SizeSplit<impl Stream<Item = Result<Bytes, E>> + Send>, E> | ||
| where | ||
| S: Stream<Item = Result<Bytes, E>> + Send, | ||
| E: Send, | ||
| { | ||
| let (head, stream) = peek_n(stream, limit).await?; | ||
| if head.len() < limit { | ||
| Ok(SizeSplit::Small(head)) | ||
| } else { | ||
| Ok(SizeSplit::Large(stream)) | ||
| } | ||
| } |
There was a problem hiding this comment.
I believe you can simplify the API by changing peek_n to return (Bytes, Option<Stream>), i.e. return None for the stream if there's nothing after head. reject_if_compressed can re-use the bytes obtained by the first call to peek_n.
There was a problem hiding this comment.
Talked offline and while this would be really nice the issue is that if the initial peek peeks less bytes than what we need eventually this will not work. Which is the case since we want to roll this out with a peek of 0 bytes initially.
Co-authored-by: Joris Bayer <joris.bayer@sentry.io>
|
For the bytes we get from |
tobias-wilfert
changed the title
|
There are a bunch of merge conflicts I will still need to fix. |
…/inline-small-attachments
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 1d7a731. Configure here.
2 participants
Fix: RELAY-255