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

[Tables] Support Emulator and Azurite #13165

Merged
merged 4 commits into from
Jan 13, 2021
Merged

[Tables] Support Emulator and Azurite #13165

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4354abd
Support Emulator and Azurite
joheredi Jan 12, 2021
29d7f9e
Support UseDevelopmentStorage connection string
joheredi Jan 12, 2021
ed8a4af
make usedevelopmentstorage case-insensitive
joheredi Jan 12, 2021
ddf59bb
Use toLowerCase
joheredi Jan 13, 2021
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: 2 additions & 0 deletions sdk/tables/data-tables/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

## 1.0.0-beta.4 (2021-01-12)

- Fix issue that prevented support for Azure Storage Emulator and Azurite [#13165](https://github.com/Azure/azure-sdk-for-js/pull/13165)
HarshaNalluru marked this conversation as resolved.
Show resolved Hide resolved

### Breaking Changes

- Don't deserialize DateTime into a JavaScript Date to avoid losing precision [#12650](https://github.com/Azure/azure-sdk-for-js/pull/12650)
Expand Down
15 changes: 8 additions & 7 deletions sdk/tables/data-tables/src/TableClient.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ export class TableClient {
? credentialOrOptions
: options) || {};

clientOptions.endpoint = clientOptions.endpoint || url;
if (!clientOptions.userAgentOptions) {
clientOptions.userAgentOptions = {};
}
Expand All @@ -151,15 +152,15 @@ export class TableClient {
} else {
// The client is meant to be a regular service client, so we need to create the regular set of pipelines
const internalPipelineOptions: InternalPipelineOptions = {
...clientOptions,
...{
loggingOptions: {
logger: logger.info,
allowedHeaderNames: [...TablesLoggingAllowedHeaderNames]
}
loggingOptions: {
logger: logger.info,
allowedHeaderNames: [...TablesLoggingAllowedHeaderNames]
}
};
pipeline = createPipelineFromOptions(internalPipelineOptions, credential);
pipeline = {
...clientOptions,
...createPipelineFromOptions(internalPipelineOptions, credential)
};
}

this.tableName = tableName;
Expand Down
16 changes: 9 additions & 7 deletions sdk/tables/data-tables/src/TableServiceClient.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,8 @@ export class TableServiceClient {
? credentialOrOptions
: options) || {};

clientOptions.endpoint = clientOptions.endpoint || url;

if (!clientOptions.userAgentOptions) {
clientOptions.userAgentOptions = {};
}
Expand All @@ -114,16 +116,16 @@ export class TableServiceClient {
}

const internalPipelineOptions: InternalPipelineOptions = {
...clientOptions,
...{
loggingOptions: {
logger: logger.info,
allowedHeaderNames: [...TablesLoggingAllowedHeaderNames]
}
loggingOptions: {
logger: logger.info,
allowedHeaderNames: [...TablesLoggingAllowedHeaderNames]
}
};

const pipeline = createPipelineFromOptions(internalPipelineOptions, credential);
const pipeline = {
...clientOptions,
...createPipelineFromOptions(internalPipelineOptions, credential)
};
const client = new GeneratedClient(url, pipeline);
this.table = client.table;
this.service = client.service;
Expand Down
6 changes: 6 additions & 0 deletions sdk/tables/data-tables/src/utils/connectionString.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ import { fromAccountConnectionString, getAccountConnectionString } from "./accou
import { ClientParamsFromConnectionString, ConnectionString } from "./internalModels";
import { URL } from "./url";

const DevelopmentConnectionString =
"DefaultEndpointsProtocol=http;AccountName=devstoreaccount1;AccountKey=Eby8vdM02xNOcqFlqUwJPLlmEtlCDXJ1OUzFT50uSRZ6IFsuFq2UVErCz4I6tq/K1SZFPTOtr/KBHBeksoGMGw==;TableEndpoint=http://127.0.0.1:10002/devstoreaccount1";
Copy link
Member Author

Choose a reason for hiding this comment

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

This is a publicly available connection string used by Azurite and Azure Storage Emulator

References:
https://github.com/Azure/Azurite/tree/table#default-storage-account
https://docs.microsoft.com/en-us/azure/storage/common/storage-use-emulator#connect-to-the-emulator-account-using-the-shortcut

Copy link

Choose a reason for hiding this comment

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

The emulator may not be hosted locally, in which case this will fail. https://docs.microsoft.com/en-us/azure/storage/common/storage-use-azurite#command-line-options

Copy link

Choose a reason for hiding this comment

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

It may also have a differently configured account name and key https://docs.microsoft.com/en-us/azure/storage/common/storage-use-azurite#authorization-for-tools-and-sdks

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, it looks like in those more advanced scenarios customers may want to use an actual connection string instead of the development shortcut.

We will need to document this in the samples to make sure there is no confusion.

Makes sense? Or do you have something different in mind?

Copy link

@riverar riverar Jan 12, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I'm testing this right now but I think the shortcut points to different targets based on environment configuration. For example, if AZURITE_ACCOUNTS exists, it may change the target of the shortcut (becoming unaligned with your hardcoded value) and produce a confusing credential mismatch error when the user hasn't specified any credentials.

Copy link

Choose a reason for hiding this comment

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

Agreed.


/**
* This function parses a connection string into a set of
* parameters to pass to be passed to TableClientService,
Expand All @@ -20,6 +23,9 @@ export function getClientParamsFromConnectionString(
connectionString: string,
options?: TableServiceClientOptions
): ClientParamsFromConnectionString {
if (connectionString.toLocaleLowerCase().indexOf("usedevelopmentstorage=true") !== -1) {
Copy link
Member

Choose a reason for hiding this comment

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

If we have to support lower case too, storage-blob and storage-queue packages would fail with the emulator.

Copy link
Member

Choose a reason for hiding this comment

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

@joheredi Can log an issue for this?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes! I filed 2 issues. This is not only for the Emulator but any connection string should have case-insensitive keys

Storage: #13173
Tables: #13172

Copy link
Member

Choose a reason for hiding this comment

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

Is using toLocaleLowerCase() here what we want vs toLowerCase()? I feel like we don't want this casing operation to be locale specific (but it probably doesn't matter for this specific example?).

Copy link
Member Author

@joheredi joheredi Jan 13, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah you are right! Good catch 😄

connectionString = DevelopmentConnectionString;
}
const extractedCreds = extractConnectionStringParts(connectionString);
if (extractedCreds.kind === "AccountConnString") {
return fromAccountConnectionString(extractedCreds, options);
Expand Down