Page MenuHomePhabricator
Differential D5093

[services] Backup - Blob Get Client - Refactor put terminate
ClosedPublic
Actions

Authored by • karol on Sep 9 2022, 4:43 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Mar 16, 12:39 AM
Unknown Object (File)
Thu, Mar 13, 8:00 PM
Unknown Object (File)
Thu, Mar 13, 2:43 AM
Unknown Object (File)
Wed, Mar 12, 1:00 PM
Unknown Object (File)
Wed, Mar 12, 5:43 AM
Unknown Object (File)
Thu, Mar 6, 5:17 AM
Unknown Object (File)
Thu, Mar 6, 5:17 AM
Unknown Object (File)
Thu, Mar 6, 5:17 AM
View All Files
Subscribers
• abosh
ashoat
atul
tomek

Details

Reviewers
• jon
varun
• max
tomek
Commits
rCOMMc0d83a91976c: [services] Backup - Blob Get Client - Refactor put terminate
Summary

Depends on D5092

Linear task: https://linear.app/comm/issue/ENG-1734/improve-error-handling-system-in-the-blob-client

Using a new way of error handling in the terminate method of the put client.

Test Plan
cd services/backup/blob_client
cargo check

after running localstack, blob and backup (in this particular order), this still works:

yarn run-performance-tests backup

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

• karol created this revision.Sep 9 2022, 4:43 AM
Herald added a reviewer: • jon. · View Herald TranscriptSep 9 2022, 4:43 AM
Herald added subscribers: • abosh, atul, tomek, ashoat. · View Herald Transcript
Harbormaster completed remote builds in B12046: Diff 16545.Sep 9 2022, 4:54 AM
• karol requested review of this revision.Sep 9 2022, 4:54 AM
• karol edited the summary of this revision. (Show Details)Sep 9 2022, 4:54 AM
• karol edited the test plan for this revision. (Show Details)
• karol added reviewers: varun, • max, tomek.
• karol added a parent revision: D5092: [services] Backup - Blob Get Client - Refactor get initialize.
• karol added a child revision: D5094: [services] Backup - Blob Get Client - Refactor put write.
• jon requested changes to this revision.Sep 9 2022, 10:52 AM
• jon added inline comments.
services/backup/blob_client/src/put_client.rs
59 ↗(On Diff #16545)

what's the purpose of holder?

also &String should always be &str in function inputs, as String and &String should be able to AsRef into a &str.

This revision now requires changes to proceed.Sep 9 2022, 10:52 AM
• karol added inline comments.Sep 12 2022, 1:26 AM
services/backup/blob_client/src/put_client.rs
59 ↗(On Diff #16545)

By holder we're identifying the client we're referring to.

Sure, we can use &str.

• karol updated this revision to Diff 16574.Sep 12 2022, 1:37 AM

update

Harbormaster completed remote builds in B12074: Diff 16574.Sep 12 2022, 1:47 AM
• jon accepted this revision.Sep 12 2022, 9:05 AM
This revision is now accepted and ready to land.Sep 12 2022, 9:05 AM
tomek requested changes to this revision.Sep 15 2022, 8:46 AM
tomek added inline comments.
services/backup/blob_client/src/put_client.rs
36 ↗(On Diff #16574)

What does this comment mean?

59 ↗(On Diff #16574)

D5090 introduces is_initialized_new(holder: &String) function, later in the stack in D5092 this function is deleted and is_initialized(holder: &str) is used instead, but then this diff introduces is_initialized_new again but with a different parameter type. Something is wrong here.

This revision now requires changes to proceed.Sep 15 2022, 8:46 AM
• karol added inline comments.Sep 19 2022, 1:29 AM
services/backup/blob_client/src/put_client.rs
36 ↗(On Diff #16574)

should be removed

59 ↗(On Diff #16574)

You missed that the 2 diffs you mentioned do changes in get client and this diff does changes in put client. So I don't put something back again really.

• karol updated this revision to Diff 16810.Sep 19 2022, 1:31 AM

update

Harbormaster completed remote builds in B12249: Diff 16810.Sep 19 2022, 1:39 AM
tomek accepted this revision.Sep 19 2022, 5:07 AM
tomek added inline comments.
services/backup/blob_client/src/put_client.rs
59 ↗(On Diff #16574)

Ahhh, ok. Sorry for the confusion!

This revision is now accepted and ready to land.Sep 19 2022, 5:07 AM
• karol updated this revision to Diff 16842.Sep 19 2022, 7:26 AM

rebase

This revision was landed with ongoing or failed builds.Sep 19 2022, 7:28 AM
Closed by commit rCOMMc0d83a91976c: [services] Backup - Blob Get Client - Refactor put terminate (authored by • karol). · Explain Why
This revision was automatically updated to reflect the committed changes.
• karol added a commit: rCOMMc0d83a91976c: [services] Backup - Blob Get Client - Refactor put terminate.
Harbormaster completed remote builds in B12280: Diff 16842.Sep 19 2022, 7:36 AM

Revision Contents
Changeset List

PathSize
services/
backup/
blob_client/
src/
45 lines

Diff 16842

View Options

services/backup/blob_client/src/put_client.rs

// @flow // @flow
import * as React from 'react'; import * as React from 'react';
import { Text } from 'react-native'; import { Text } from 'react-native';
import { oldValidUsernameRegexString } from 'lib/shared/account-utils.js'; import { oldValidUsernameRegexString } from 'lib/shared/account-utils.js';
import { import {
getNewTextAndSelection, getNewTextAndSelection,
encodeChatMentionText, encodeChatMentionText,
type Selection, type Selection,
type TypeaheadTooltipActionItem, type TypeaheadTooltipActionItem,
type MentionTypeaheadSuggestionItem, type MentionTypeaheadSuggestionItem,
} from 'lib/shared/mention-utils.js'; } from 'lib/shared/mention-utils.js';
import { validChatNameRegexString } from 'lib/shared/thread-utils.js';
import { stringForUserExplicit } from 'lib/shared/user-utils.js'; import { stringForUserExplicit } from 'lib/shared/user-utils.js';
import ThreadAvatar from '../avatars/thread-avatar.react.js';
import UserAvatar from '../avatars/user-avatar.react.js'; import UserAvatar from '../avatars/user-avatar.react.js';
import type { TypeaheadTooltipStyles } from '../chat/typeahead-tooltip.react.js'; import type { TypeaheadTooltipStyles } from '../chat/typeahead-tooltip.react.js';
// Native regex is a little bit different than web one as // Native regex is a little bit different than web one as
// there are no named capturing groups yet on native. // there are no named capturing groups yet on native.
const nativeMentionTypeaheadRegex: RegExp = new RegExp( const nativeMentionTypeaheadRegex: RegExp = new RegExp(
`((^(.|\n)*\\s+)|^)@(${oldValidUsernameRegexString})?$`, `((^(.|\n)*\\s+)|^)@(${oldValidUsernameRegexString}|${validChatNameRegexString})?$`,
); );
export type TypeaheadTooltipActionsParams<SuggestionType> = { export type TypeaheadTooltipActionsParams<SuggestionType> = {
+suggestions: $ReadOnlyArray<SuggestionType>, +suggestions: $ReadOnlyArray<SuggestionType>,
+textBeforeAtSymbol: string, +textBeforeAtSymbol: string,
+text: string, +text: string,
+textPrefix: string, +textPrefix: string,
+focusAndUpdateTextAndSelection: (text: string, selection: Selection) => void, +focusAndUpdateTextAndSelection: (text: string, selection: Selection) => void,
Show All 31 Lines if (suggestion.type === 'user') {
}); });
}, },
actionButtonContent: { actionButtonContent: {
type: 'user', type: 'user',
userInfo, userInfo,
}, },
}); });
} else if (suggestion.type === 'chat') { } else if (suggestion.type === 'chat') {
const { chat } = suggestion; const { threadInfo } = suggestion;
const mentionText = `@[[${chat.id}:${encodeChatMentionText( const mentionText = `@[[${threadInfo.id}:${encodeChatMentionText(
chat.uiName, threadInfo.uiName,
)}]]`; )}]]`;
actions.push({ actions.push({
key: chat.id, key: threadInfo.id,
execute: () => { execute: () => {
const { newText, newSelectionStart } = getNewTextAndSelection( const { newText, newSelectionStart } = getNewTextAndSelection(
textBeforeAtSymbol, textBeforeAtSymbol,
text, text,
textPrefix, textPrefix,
mentionText, mentionText,
); );
focusAndUpdateTextAndSelection(newText, { focusAndUpdateTextAndSelection(newText, {
start: newSelectionStart, start: newSelectionStart,
end: newSelectionStart, end: newSelectionStart,
}); });
}, },
actionButtonContent: { actionButtonContent: {
type: 'chat', type: 'chat',
chat, threadInfo,
}, },
}); });
} }
} }
return actions; return actions;
} }
export type TypeaheadTooltipButtonRendererParams<SuggestionItemType> = { export type TypeaheadTooltipButtonRendererParams<SuggestionItemType> = {
+item: TypeaheadTooltipActionItem<SuggestionItemType>, +item: TypeaheadTooltipActionItem<SuggestionItemType>,
+styles: TypeaheadTooltipStyles, +styles: TypeaheadTooltipStyles,
}; };
function mentionTypeaheadTooltipButtonRenderer({ function mentionTypeaheadTooltipButtonRenderer({
item, item,
styles, styles,
}: TypeaheadTooltipButtonRendererParams<MentionTypeaheadSuggestionItem>): React.Node { }: TypeaheadTooltipButtonRendererParams<MentionTypeaheadSuggestionItem>): React.Node {
let avatarComponent = null; let avatarComponent = null;
let typeaheadTooltipButtonText = null; let typeaheadTooltipButtonText = null;
if (item.actionButtonContent.type === 'user') { if (item.actionButtonContent.type === 'user') {
avatarComponent = ( avatarComponent = (
<UserAvatar size="small" userID={item.actionButtonContent.userInfo.id} /> <UserAvatar size="small" userID={item.actionButtonContent.userInfo.id} />
); );
typeaheadTooltipButtonText = item.actionButtonContent.userInfo.username; typeaheadTooltipButtonText = item.actionButtonContent.userInfo.username;
} else if (item.actionButtonContent.type === 'chat') {
typeaheadTooltipButtonText = item.actionButtonContent.threadInfo.uiName;
avatarComponent = (
<ThreadAvatar
size="small"
threadInfo={item.actionButtonContent.threadInfo}
/>
);
} }
return ( return (
<> <>
{avatarComponent} {avatarComponent}
<Text style={styles.buttonLabel} numberOfLines={1}> <Text style={styles.buttonLabel} numberOfLines={1}>
{typeaheadTooltipButtonText} {typeaheadTooltipButtonText}
</Text> </Text>
</> </>
); );
} }
export { export {
nativeMentionTypeaheadRegex, nativeMentionTypeaheadRegex,
mentionTypeaheadTooltipActions, mentionTypeaheadTooltipActions,
mentionTypeaheadTooltipButtonRenderer, mentionTypeaheadTooltipButtonRenderer,
}; };