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

Enable gzip compression for Sync PATCH payloads #2805

Merged
merged 7 commits into from
May 10, 2024

Conversation

ayoy
Copy link
Contributor

@ayoy ayoy commented Apr 30, 2024

Task/Issue URL: https://app.asana.com/0/0/1206919211758354/f

Description:
This change adds gzip compression to all Sync data PATCH requests.

Steps to test this PR:

  1. Run both iOS and macOS apps from branches in linked PRs.
  2. Connect devices with Sync and smoke test syncing data.

Internal references:

Software Engineering Expectations
Technical Design Template

@ayoy ayoy self-assigned this Apr 30, 2024
@ayoy ayoy requested a review from bwaresiak April 30, 2024 07:18
@ayoy ayoy assigned bwaresiak and unassigned ayoy Apr 30, 2024
@ayoy ayoy marked this pull request as ready for review April 30, 2024 07:18
@ayoy ayoy removed the Do Not Merge label May 10, 2024
@ayoy ayoy requested a review from SabrinaTardio May 10, 2024 04:17
@ayoy ayoy assigned SabrinaTardio and unassigned bwaresiak May 10, 2024
Copy link
Contributor

@SabrinaTardio SabrinaTardio left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -63,6 +63,8 @@ public final class SyncSettingsAdapter {
syncErrorCancellable = provider.syncErrorPublisher
.sink { error in
switch error {
case SyncError.patchPayloadCompressionFailed(let errorCode):
Copy link
Contributor

Choose a reason for hiding this comment

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

I will move it into SyncErrorHandler when I got some time

@ayoy ayoy merged commit 27e7f4d into main May 10, 2024
14 checks passed
@ayoy ayoy deleted the dominik/sync-payload-compression branch May 10, 2024 13:12
samsymons added a commit that referenced this pull request May 10, 2024
# By amddg44 (3) and others
# Via GitHub (1) and amddg44 (1)
* main:
  Enable gzip compression for Sync PATCH payloads (#2805)
  sync error notify (#2822)
  Wire up VPN error UI (#2826)
  Mergeback from release/7.119.0
  Release 7.119.0-1 (#2840)
  Add temporary survey to Passwords screen (#2834)
  Upload logs on cancel (#2836)
  Add bookmarks to top hits suggestions on iOS (#2835)
  Fix Maestro reporting for e2e tests (#2713)
  support marketplace kit (#2833)
  Enable attribution flag (#2825)
  Release 7.119.0-0 (#2831)
  Fix subscriptions glitch in Settings (#2828)
  Delegate SecureVault KeyStore Events in All Scenarios  (#2827)
  Clears subscription cache on restore (#2823)

# Conflicts:
#	DuckDuckGo.xcodeproj/project.pbxproj
#	DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved
samsymons added a commit that referenced this pull request May 13, 2024
* main: (27 commits)
  Release 7.120.0-0 (#2846)
  Enable gzip compression for Sync PATCH payloads (#2805)
  sync error notify (#2822)
  Wire up VPN error UI (#2826)
  Mergeback from release/7.119.0
  Release 7.119.0-1 (#2840)
  Add temporary survey to Passwords screen (#2834)
  Upload logs on cancel (#2836)
  Add bookmarks to top hits suggestions on iOS (#2835)
  Fix Maestro reporting for e2e tests (#2713)
  support marketplace kit (#2833)
  Enable attribution flag (#2825)
  Release 7.119.0-0 (#2831)
  Fix subscriptions glitch in Settings (#2828)
  Delegate SecureVault KeyStore Events in All Scenarios  (#2827)
  Clears subscription cache on restore (#2823)
  Enable field validation for Sync payloads (#2807)
  Fix up address bar spoofing tests to match new behaviours. (#2743)
  Release 7.118.0-4 (#2821)
  Settings: onAppear changed to onDisappear (#2817) (#2820)
  ...
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.

3 participants