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

feat: store remote-settings attachments in local storage #6490

Merged
merged 1 commit into from
Nov 22, 2024

Conversation

gruberb
Copy link
Member

@gruberb gruberb commented Nov 22, 2024

Solves: https://bugzilla.mozilla.org/show_bug.cgi?id=1929659

Hooking up attachments to the sqlite storage in the remote-settings component.

I also changed the name of the Attachment struct to AttachmentMetadata. I tried to do the same for the RemoteSettingsRecord, but we had to do some serialziation magic, and it seemed to cumbersome just for renaming things.

The idea is to be intentional that what's part of the RemoteSettingsRecord is not the attachment itself, but just the metadata for an attachment. The Remote Settings server response is calling it attachment, and therefore we adopt the name here as well.

Copy link
Contributor

@bendk bendk left a comment

Choose a reason for hiding this comment

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

Looks great, the only potential issue is the name change. I'm going to approve, but let's test building the consumer apps before merging this one.

components/remote_settings/src/client.rs Outdated Show resolved Hide resolved
@gruberb gruberb added this pull request to the merge queue Nov 22, 2024
@gruberb gruberb removed this pull request from the merge queue due to a manual request Nov 22, 2024
@gruberb gruberb added this pull request to the merge queue Nov 22, 2024
Merged via the queue into main with commit 8eb760d Nov 22, 2024
15 checks passed
@gruberb gruberb deleted the store-attachments branch November 22, 2024 21:31
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.

2 participants