services/backup/blob_client/src/get_client.rs
131–137 ↗	(On Diff #16175)	we pulled `client` out of the mutex guard, and now we are re-assigning it? why?
132–136 ↗	(On Diff #16175)	This should be a similar way to express this.

This revision now requires changes to proceed.Sep 1 2022, 9:11 AM

• karol added inline comments.Sep 5 2022, 12:06 AM

services/backup/blob_client/src/get_client.rs
131–137 ↗	(On Diff #16175)	Rust will not allow modifying struct members so we have to `take()` and re-put again.
132–136 ↗	(On Diff #16175)	Yes, looks okay, nice catch, thanks!

tomek requested changes to this revision.Sep 5 2022, 5:37 AM

tomek added inline comments.

services/backup/blob_client/src/get_client.rs
131–137 ↗	(On Diff #16175)	I'm not sure if this is correct in general. For example, you can use `&mut` reference...
135 ↗	(On Diff #16175)	Is there a way to avoid mutating the data from outside this block? Can we use `block_on` return value for this?

• karol added inline comments.Sep 6 2022, 2:43 AM

services/backup/blob_client/src/get_client.rs
131–137 ↗	(On Diff #16175)	https://linear.app/comm/issue/ENG-1736/try-to-avoid-reassigning-when-modifying-the-field-of-a-global-struct
135 ↗	(On Diff #16175)	yes

address comments

Harbormaster completed remote builds in B11910: Diff 16353.Sep 6 2022, 2:52 AM

tomek accepted this revision.Sep 7 2022, 5:15 AM

tomek added inline comments.

services/backup/blob_client/src/get_client.rs
135 ↗	(On Diff #16175)	Nice!

Will address concerns in https://linear.app/comm/issue/ENG-1736/try-to-avoid-reassigning-when-modifying-the-field-of-a-global-struct

This revision is now accepted and ready to land.Sep 7 2022, 11:16 AM

rebase

This revision was landed with ongoing or failed builds.Sep 8 2022, 4:10 AM

Closed by commit rCOMM0edcf73c41af: [services] Backup - Blob Get Client - Add blocking read (authored by • karol). · Explain Why

This revision was automatically updated to reflect the committed changes.

• karol added a commit: rCOMM0edcf73c41af: [services] Backup - Blob Get Client - Add blocking read.

Harbormaster completed remote builds in B11992: Diff 16447.Sep 8 2022, 4:17 AM

Revision Contents
Changeset List

Path

Size

services/

backup/

blob_client/

src/

get_client.rs

19 lines

Diff 16473

View Options

services/backup/blob_client/src/get_client.rs

	// @flow			// @flow

	import type { ResponseFailure } from '@parse/node-apn';			import type { ResponseFailure } from '@parse/node-apn';
	import type { FirebaseApp, FirebaseError } from 'firebase-admin';			import type { FirebaseApp, FirebaseError } from 'firebase-admin';
	import invariant from 'invariant';			import invariant from 'invariant';
	import nodeFetch from 'node-fetch';			import nodeFetch from 'node-fetch';
	import type { Response } from 'node-fetch';			import type { Response } from 'node-fetch';
				import uuid from 'uuid';
	import webpush from 'web-push';			import webpush from 'web-push';

	import type { PlatformDetails } from 'lib/types/device-types.js';			import type { PlatformDetails } from 'lib/types/device-types.js';
	import { threadSubscriptions } from 'lib/types/subscription-types.js';			import { threadSubscriptions } from 'lib/types/subscription-types.js';
	import { threadPermissions } from 'lib/types/thread-permission-types.js';			import { threadPermissions } from 'lib/types/thread-permission-types.js';

				import { encryptBlobPayload } from './crypto.js';
	import {			import {
	getAPNPushProfileForCodeVersion,			getAPNPushProfileForCodeVersion,
	getFCMPushProfileForCodeVersion,			getFCMPushProfileForCodeVersion,
	getAPNProvider,			getAPNProvider,
	getFCMProvider,			getFCMProvider,
	ensureWebPushInitialized,			ensureWebPushInitialized,
	getWNSToken,			getWNSToken,
	} from './providers.js';			} from './providers.js';
	▲ Show 20 Lines • Show All 362 Lines • ▼ Show 20 Lines

	async function blobServiceUpload(payload: string): Promise<			async function blobServiceUpload(payload: string): Promise<
	\| {			\| {
	+blobHash: string,			+blobHash: string,
	+encryptionKey: string,			+encryptionKey: string,
	}			}
	\| { +blobUploadError: string },			\| { +blobUploadError: string },
	> {			> {
	return upload(payload);			const blobHolder = uuid.v4();
				try {
				const { encryptionKey, encryptedPayload, encryptedPayloadHash } =
				await encryptBlobPayload(payload);
				await upload(encryptedPayload, encryptedPayloadHash, blobHolder);
				return {
				blobHash: encryptedPayloadHash,
				encryptionKey,
				};
				} catch (e) {
				return {
				blobUploadError: e.message,
				};
				}
	}			}

	export {			export {
	apnPush,			apnPush,
	blobServiceUpload,			blobServiceUpload,
	fcmPush,			fcmPush,
	webPush,			webPush,
	wnsPush,			wnsPush,
	getUnreadCounts,			getUnreadCounts,
	apnMaxNotificationPayloadByteSize,			apnMaxNotificationPayloadByteSize,
	fcmMaxNotificationPayloadByteSize,			fcmMaxNotificationPayloadByteSize,
	wnsMaxNotificationPayloadByteSize,			wnsMaxNotificationPayloadByteSize,
	};			};

[services] Backup - Blob Get Client - Add blocking readClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 16473

services/backup/blob_client/src/get_client.rs

[services] Backup - Blob Get Client - Add blocking read
ClosedPublic
Actions

Revision Contents
Changeset List