Paths

Table of Contentst

Differential D13878

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

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

Tags

None

Referenced Files

	Unknown Object (File)
	Fri, Dec 20, 9:01 PM

	Unknown Object (File)
	Thu, Dec 19, 2:20 AM

	Unknown Object (File)
	Wed, Dec 18, 7:28 PM

	Unknown Object (File)
	Mon, Dec 16, 4:45 PM

	Unknown Object (File)
	Mon, Dec 16, 3:40 PM

	Unknown Object (File)
	Mon, Dec 16, 10:37 AM

	Unknown Object (File)
	Mon, Dec 9, 10:11 PM

	Unknown Object (File)
	Mon, Dec 9, 8:06 PM

View All 48 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.Nov 5 2024, 9:10 AM

kamil held this revision as a draft.

Herald added subscribers: tomek, ashoat. · View Herald TranscriptNov 5 2024, 9:10 AM

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

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

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

minor update

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

kamil published this revision for review.Nov 6 2024, 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.Nov 6 2024, 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.Nov 7 2024, 12:38 AM

This revision is now accepted and ready to land.Nov 7 2024, 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.Nov 7 2024, 2:30 AM

Closed by commit rCOMM8561f8e86b3e: [backup-service] make `user_data` and `user_keys` optional in `BackupData` (authored by kamil). · Explain WhyNov 7 2024, 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...