Paths

Table of Contentst

Differential D13878

[backup-service] make `user_data` and `user_keys` optional in `BackupData`
ClosedPublic
Actions

Authored by kamil on Tue, Nov 5, 9:10 AM.

Tags

None

Referenced Files

	F3348346: D13878.id45634.diff
	Fri, Nov 22, 2:32 PM

	Unknown Object (File)
	Wed, Nov 20, 4:53 PM

	Unknown Object (File)
	Wed, Nov 20, 5:09 AM

	Unknown Object (File)
	Sun, Nov 17, 9:06 PM

	Unknown Object (File)
	Fri, Nov 15, 12:10 PM

	Unknown Object (File)
	Fri, Nov 15, 10:58 AM

	Unknown Object (File)
	Thu, Nov 14, 1:14 AM

	Unknown Object (File)
	Wed, Nov 13, 1:51 AM

View All 30 Files

Subscribers

Details

Reviewers

varun
bartek

Commits

rCOMM8561f8e86b3e: [backup-service] make `user_data` and `user_keys` optional in `BackupData`

Summary

Part of ENG-9673.

Tracked in ENG-9649.

Depends on D13877

Test Plan

Tested in D13879

Diff Detail

Repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

kamil created this revision.Tue, Nov 5, 9:10 AM

kamil held this revision as a draft.

Herald added subscribers: tomek, ashoat. · View Herald TranscriptTue, Nov 5, 9:10 AM

kamil added a child revision: D13879: [backup-service][commtest] implement integration tests for uploading User Keys and User Data separately.Tue, Nov 5, 9:12 AM

kamil edited the test plan for this revision. (Show Details)Tue, Nov 5, 9:17 AM

Harbormaster failed remote builds in B32545: Diff 45613!Tue, Nov 5, 9:45 AM

minor update

Harbormaster completed remote builds in B32557: Diff 45634.Wed, Nov 6, 1:21 AM

kamil published this revision for review.Wed, Nov 6, 3:45 AM

kamil added inline comments.

native/native_rust_library/src/backup/upload_handler.rs
199–209 ↗	(On Diff #45634)	Depending on what files will be generated we're going to upload User Keys, or User Data, or both
services/commtest/tests/backup_integration_test.rs
60 ↗	(On Diff #45634)	not sure what is best pattern for this
shared/backup_client/src/lib.rs
59–84 ↗	(On Diff #45634)	this is strange but we need to maintain order, otherwise backup service can fail to parse Multiplart
86–88 ↗	(On Diff #45634)	We always want to upload at least one, and depending on data we're gone chose endpoint (see below)

bartek added inline comments.Wed, Nov 6, 9:40 PM

services/commtest/tests/backup_integration_test.rs
60 ↗	(On Diff #45634)	This is good
shared/backup_client/src/lib.rs
59–84 ↗	(On Diff #45634)	This is normal - Backup service is designed in such a way that expects these fields in order. So is Blob service.
86–96 ↗	(On Diff #45634)	I'd also put this above creation of `Form::new()` etc to early exit when data and keys are both none

bartek accepted this revision.Thu, Nov 7, 12:38 AM

This revision is now accepted and ready to land.Thu, Nov 7, 12:38 AM

address review

shared/backup_client/src/lib.rs
86–96 ↗	(On Diff #45634)	Good call, thanks

Harbormaster completed remote builds in B32566: Diff 45652.Thu, Nov 7, 2:30 AM

Closed by commit rCOMM8561f8e86b3e: [backup-service] make `user_data` and `user_keys` optional in `BackupData` (authored by kamil). · Explain WhyThu, Nov 7, 4:07 AM

This revision was automatically updated to reflect the committed changes.

kamil added a commit: rCOMM8561f8e86b3e: [backup-service] make `user_data` and `user_keys` optional in `BackupData`.

Revision Contents
Changeset List

Path

Size

native/

native_rust_library/

src/

backup/

upload_handler.rs

13 lines

services/

commtest/

src/

backup/

backup_utils.rs

8 lines

tests/

backup_integration_test.rs

6 lines

backup_performance_test.rs

4 lines

shared/

backup_client/

src/

48 lines

Diff 45682

native/native_rust_library/src/backup/upload_handler.rs

Loading...

services/commtest/src/backup/backup_utils.rs

Loading...

services/commtest/tests/backup_integration_test.rs

Loading...

services/commtest/tests/backup_performance_test.rs

Loading...

shared/backup_client/src/lib.rs

Loading...