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.

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

[Storage] Name properties on clients - Emulator support #5557

Merged
merged 8 commits into from Oct 17, 2019

Conversation

HarshaNalluru
Copy link
Member

@HarshaNalluru HarshaNalluru commented Oct 14, 2019

This PR allows parsing the names from a url meant for emulators in storage-blob and storage-queue.

  • containerName and blobName for blob
  • queueName for queue

@HarshaNalluru HarshaNalluru added Client This issue points to a problem in the data-plane of the library. Storage Storage Service (Queues, Blobs, Files) labels Oct 14, 2019
@HarshaNalluru HarshaNalluru changed the title Emulator support [Storage] Name properties on clients - Emulator support Oct 14, 2019
@HarshaNalluru HarshaNalluru changed the base branch from feature/storage to master October 16, 2019 19:10
@HarshaNalluru
Copy link
Member Author

HarshaNalluru commented Oct 16, 2019

After the changes in this PR, storage packages currently support the following

  • the emulator url when the blobEndpoint is http://127.0.0.1:10000/devstoreaccount1
  • the emulator url when the queueEndpoint is http://127.0.0.1:10001/devstoreaccount1
  • emulator connection string shorthand also works
    • UseDevelopmentStorage=true
    • (with proxyURI) UseDevelopmentStorage=true;DevelopmentStorageProxyUri=http://127.0.0.1:10001

and

  • [Major Use Case/Scenario] any url or a connection string that is provided for a storage account by the portal.

@XiaoningLiu mentioned that we need to add support for the following cases

  • endpoint is localhost, or other single word domain without any dot in the endpoint
  • IP addresses (v4 or v6) - For (Dev) testing environments

Considering the support to be added is non-breaking, @jeremymeng and @ramya-rao-a suggested that we do the updates after the p.5 release.

So, I believe this PR is safe to merge.

I'll log an issue for the "TO DO - Add more support" which would be picked up post p.5.

Edit - Logged an issue #5604

Copy link
Contributor

@jeremymeng jeremymeng left a comment

Choose a reason for hiding this comment

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

LGTM

let urlWithoutSAS = this.url.split("?")[0]; // removing the sas part of url if present
urlWithoutSAS = urlWithoutSAS.endsWith("/") ? urlWithoutSAS.slice(0, -1) : urlWithoutSAS; // Slicing off '/' at the end if exists

const partsOfUrl = urlWithoutSAS.match("([^/]*)://([^/]*)/([^/]*)(/(.*))?");
// http://127.0.0.1:10000/devstoreaccount1
Copy link
Member

Choose a reason for hiding this comment

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

IP style URL is not fixed to 127.0.0.1. Besides Azurite, Azure Storage test servers are also using IP style url/endpoints, but IP is not fixed. For example, https://52.132.33.11:81/accountname. Or even IPv6 style in the future. Alghorithm in SDK should cover these scenarios.

Some non IP style url like http://localhost:10000/accountname also have account name in the path. Or some devs may define the host file pointing to a local deployed xstore service like http://customized.server:10000/accountname

It's bit complex to come up with an alghorithm to cover all scenarios in the SDK side though.

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, logged an issue here #5604.
Since the new changes would only be additional support, we are postponing and will most likely be done after the p.5.

@HarshaNalluru HarshaNalluru merged commit 3742ab7 into Azure:master Oct 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Client This issue points to a problem in the data-plane of the library. Storage Storage Service (Queues, Blobs, Files)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants