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.

Sign up for GitHub

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

Update DTFx.AzureStorage v2 Queue to use Base64 Encoding #1154

Closed
wants to merge 8 commits into from
+74 −12
Closed

Update DTFx.AzureStorage v2 Queue to use Base64 Encoding #1154

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
1095b9a
Update StorageServiceClientProvider.cs
nytian Aug 7, 2024
0e2b1bc
Update DurableTask.AzureStorage.csproj
nytian Aug 7, 2024
5614986
add unit test
nytian Aug 8, 2024
db12a59
refactor method and update test name
nytian Aug 8, 2024
a92d9b6
update test
nytian Aug 9, 2024
a65f556
add encoding check for different strategy at storage queue option
nytian Aug 12, 2024
d68a8ae
add comments
nytian Aug 13, 2024
033b026
enable queue client option at storageaccountclientprovider
nytian Aug 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/DurableTask.AzureStorage/DurableTask.AzureStorage.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
<PropertyGroup>
<MajorVersion>2</MajorVersion>
<MinorVersion>0</MinorVersion>
<PatchVersion>0</PatchVersion>
<PatchVersion>1</PatchVersion>
<VersionPrefix>$(MajorVersion).$(MinorVersion).$(PatchVersion)</VersionPrefix>
<FileVersion>$(VersionPrefix).0</FileVersion>
<!-- FileVersionRevision is expected to be set by the CI. This is useful for distinguishing between multiple builds of the same version. -->
Expand Down
10 changes: 8 additions & 2 deletions src/DurableTask.AzureStorage/StorageServiceClientProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,9 +116,11 @@ public static IStorageServiceClientProvider<QueueServiceClient, QueueClientOptio
throw new ArgumentNullException(nameof(connectionString));
}

options = options ?? new QueueClientOptions();
options.MessageEncoding = QueueMessageEncoding.Base64;
Copy link
Collaborator

Choose a reason for hiding this comment

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

@nytian: I usually like to follow the "rule" that, if we're doing the same work in 3 places or more, we should probably refactor that logic into a common helper function.

Otherwise, I see the following risk: we could end up adding a new method to create a queue client, and then forget to add this "base64" encoding trick.

Can you look into a refactoring of these 3 methods where they call common logic to ensure the client options is always populated with base64 encoding?

return new DefaultStorageServiceClientProvider<QueueServiceClient, QueueClientOptions>(
o => new QueueServiceClient(connectionString, o),
options ?? new QueueClientOptions());
options);
}

/// <summary>
Expand All @@ -140,6 +142,8 @@ public static IStorageServiceClientProvider<QueueServiceClient, QueueClientOptio
TokenCredential tokenCredential,
QueueClientOptions? options = null)
{
options = options ?? new QueueClientOptions();
options.MessageEncoding = QueueMessageEncoding.Base64;
return ForQueue(CreateDefaultServiceUri(accountName, "queue"), tokenCredential, options);
}

Expand All @@ -163,9 +167,11 @@ public static IStorageServiceClientProvider<QueueServiceClient, QueueClientOptio
throw new ArgumentNullException(nameof(tokenCredential));
}

options = options ?? new QueueClientOptions();
options.MessageEncoding = QueueMessageEncoding.Base64;
return new DefaultStorageServiceClientProvider<QueueServiceClient, QueueClientOptions>(
o => new QueueServiceClient(serviceUri, tokenCredential, o),
options ?? new QueueClientOptions());
options);
}

#endregion
Expand Down
Loading