Skip to content

Creating new PR for Storage Blob migration guide#15749

Merged
jaschrep-msft merged 11 commits intoAzure:masterfrom
rishabpoh:master
Oct 23, 2020
Merged

Creating new PR for Storage Blob migration guide#15749
jaschrep-msft merged 11 commits intoAzure:masterfrom
rishabpoh:master

Conversation

@rishabpoh
Copy link
Copy Markdown
Contributor

I've addressed most of the comments from the old PR. Couldn't merge correctly because I deleted my forked copy of the repo. Requesting assistance on some TODO sections.

@ghost ghost added the Storage Storage Service (Queues, Blobs, Files) label Oct 6, 2020
@rishabpoh rishabpoh changed the title Creating new PR for migration guide Creating new PR for Storage Blob migration guide Oct 6, 2020
Comment thread sdk/storage/Azure.Storage.Blobs/AzureStorageNetMigrationV12.md
```

v12
```csharp
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@amnguye can you add an additional sample to this document for when the new SAS APIs get released? Just like a note that 12.x and above users can use the following code, which may be more familiar to v11 users.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

not for this PR, but just whenever the SAS API additions get merged in.

await service.GetPropertiesAsync();
```

Summary:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What is this meant to be?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It was meant to be a brief description of the underlying changes we made to give the developer a better idea of the differences they're looking at. But it could be unnecessary too. If the summaries don't add much value then it doesn't make sense to keep them. What do you think?


In the interest of simplifying the API surface we've made a three top level clients that can be used to interact with a majority of your resources: `BlobServiceClient`, `BlobContainerClient`, and `BlobClient`.

[//]: # (Blob Metadata, properties, and attributes...)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What is this doing? I don't see anything rendering in the rich view.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It's how you leave a comment in Markdown apparently. I wanted to leave that comment as a reminder to add some migration info on that topic.

Comment thread sdk/storage/Azure.Storage.Blobs/AzureStorageNetMigrationV12.md
@amnguye
Copy link
Copy Markdown
Member

amnguye commented Oct 6, 2020

Maybe for a different PR but we should try to have the guide take snippets from actual samples (for v12). That way we know when and if our snippets ever break.

Comment on lines +161 to +168
```csharp
// Create a BlobServiceClient object which will be used to create a container client
BlobServiceClient blobServiceClient = new BlobServiceClient(connectionString);
// Create a unique name for the container
string containerName = "yourcontainer";
// Create the container and return a container client object
BlobContainerClient containerClient = await blobServiceClient.CreateBlobContainerAsync(containerName);
```
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It suggestion syntax not happy with nested code blocks. Below is a suggested change where the side-by-side examples have the same code structure. Then a shortcut method is introduced.

// Create a BlobServiceClient object which will be used to create a container client
BlobServiceClient blobServiceClient = new BlobServiceClient(connectionString);
BlobContainerClient containerClient = blobServiceClient.GetBlobContainerClient("yourcontainer");
await containerClient.CreateAsync()

Alternatively, you can skip a step in v12 by with BlobServiceClient.CreateBlobContainer():

// Create a BlobServiceClient object which will be used to create a container client
BlobServiceClient blobServiceClient = new BlobServiceClient(connectionString);
BlobContainerClient containerClient = await blobServiceClient.CreateBlobContainerAsync("yourcontainer");

BlobContainerClient containerClient = await blobServiceClient.CreateBlobContainerAsync(containerName);
```

Summary: In v11, a `CloudBlobClient` was required to get a reference to the desired blob container. In v12, the intermediate step `cloudBlobClient.GetContainerReference("yourcontainer")` was removed.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This wasn't removed. This should convey that the old methods have exact v12 counterparts.

Comment thread sdk/storage/Azure.Storage.Blobs/AzureStorageNetMigrationV12.md Outdated
uploadFileStream.Close();
```

Summary: In v11, a file path was used to upload a blob. In v12, `Stream` is used to upload a blob's content.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

streams and file paths can be used in both

Comment on lines +214 to +219
BlobDownloadInfo download = await blobClient.DownloadAsync();
using (FileStream downloadFileStream = File.OpenWrite(downloadFilePath))
{
await download.Content.CopyToAsync(downloadFileStream);
downloadFileStream.Close();
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We have download to file. use similar samples

}
```

### Listing Blobs in a Container
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We didn't force people to manually iterate over pages in v11. We just had separate methods for a lazy enumerable vs getting each page on your own. I think it's actually important to highlight both mechanisms for this section, because manually handling individual pages is a bit complex in v12, and we'll want to give a direct migration for that as well.


### Authentication

#### Managed Identity
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm not sure there's a short sample we can provide for this since it's not specific to v11 to v12 and more in the identity libraries. I think a complex stand alone solution/tutorial using MSI might be more helpful (since it requires step by step instructions on how to set up managed identity which is outside the scope of storage.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe we should provide a sample for authenticating with AAD (e.g. TokenCredential) instead

```

v12
```csharp
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

// Create BlobSasBuilder and specify parameters
BlobSasBuilder sasBuilder = new BlobSasBuilder()
{
    BlobContainerName = containerName,
    BlobName = blobName,
    ExpiresOn = new DateTimeOffset
    IPRange = new SasIPRange(System.Net.IPAddress.None, System.Net.IPAddress.None)
};
// Set Permissions
sasBuilder.SetPermissions(BlobSasPermissions.Read);

// Create SasQueryParameters
BlobSasQueryParameters parameters = sasBuilder.ToSasQueryParameters(sharedKeyCredential);

// Create Uri with SasToken
BlobUriBuilder uriBuilder = new BlobUriBuilder(blobUri)
{
    Sas = sasBuilder.ToSasQueryParameters(constants.Sas.SharedKeyCredential)
};
Uri sasUri = uriBuilder.ToUri()

Copy link
Copy Markdown
Member

@amnguye amnguye Oct 8, 2020

Choose a reason for hiding this comment

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

This is to create a service SAS. Did we also want to add one for user delegation sas?
For UserDelegation Sas it is essentially the same snippet with the exception of this line
BlobSasQueryParameters parameters = sasBuilder.ToSasQueryParameters(sharedKeyCredential);
should be changed to
BlobSasQueryParameters parameters = sasBuilder.ToSasQueryParameters(userDelegationKey, accountName);

There are various SAS tokens that may be generated. Visit our documentation pages to learn how to [Create a User Delegation SAS](https://docs.microsoft.com/azure/storage/blobs/storage-blob-user-delegation-sas-create-dotnet), [Create a Service SAS](https://docs.microsoft.com/azure/storage/blobs/storage-blob-service-sas-create-dotnet), or [Create an Account SAS](https://docs.microsoft.com/azure/storage/common/storage-account-sas-create-dotnet?toc=/azure/storage/blobs/toc.json).

v11
```csharp
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Service SAS:

    string sasBlobToken;

    // Get a reference to a blob within the container.
    // Note that the blob may not exist yet, but a SAS can still be created for it.
    CloudBlockBlob blob = container.GetBlockBlobReference(blobName);

    if (policyName == null)
    {
        // Create a new access policy and define its constraints.
        // Note that the SharedAccessBlobPolicy class is used both to define the parameters of an ad hoc SAS, and
        // to construct a shared access policy that is saved to the container's shared access policies.
        SharedAccessBlobPolicy adHocSAS = new SharedAccessBlobPolicy()
        {
            // When the start time for the SAS is omitted, the start time is assumed to be the time when the storage service receives the request.
            // Omitting the start time for a SAS that is effective immediately helps to avoid clock skew.
            SharedAccessExpiryTime = DateTime.UtcNow.AddHours(24),
            Permissions = SharedAccessBlobPermissions.Read | SharedAccessBlobPermissions.Write | SharedAccessBlobPermissions.Create
        };

        // Generate the shared access signature on the blob, setting the constraints directly on the signature.
        sasBlobToken = blob.GetSharedAccessSignature(adHocSAS);

        Console.WriteLine("SAS for blob (ad hoc): {0}", sasBlobToken);
        Console.WriteLine();
    }
    else
    {
        // Generate the shared access signature on the blob. In this case, all of the constraints for the
        // shared access signature are specified on the container's stored access policy.
        sasBlobToken = blob.GetSharedAccessSignature(null, policyName);

        Console.WriteLine("SAS for blob (stored access policy): {0}", sasBlobToken);
        Console.WriteLine();
    }

    // Return the URI string for the container, including the SAS token.
    return blob.Uri + sasBlobToken;


### Shared Access Policies

TODO
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

rishabpoh and others added 5 commits October 20, 2020 11:48
Co-authored-by: James <41338290+jaschrep-msft@users.noreply.github.com>
Co-authored-by: James <41338290+jaschrep-msft@users.noreply.github.com>
@jaschrep-msft
Copy link
Copy Markdown
Member

Am personally tracking gaps in this PR, but we need to get this baseline in.

@jaschrep-msft
Copy link
Copy Markdown
Member

/azp run net - storage - ci

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@jaschrep-msft jaschrep-msft merged commit 63f0ed1 into Azure:master Oct 23, 2020
annelo-msft pushed a commit to annelo-msft/azure-sdk-for-net that referenced this pull request Feb 17, 2021
* Adding migration guide addressing most comments from old PR

* Minor mod

* Adding migration guide addressing most comments from old PR

* Minor mod

* Fixing links

* Update sdk/storage/Azure.Storage.Blobs/AzureStorageNetMigrationV12.md

Co-authored-by: James <41338290+jaschrep-msft@users.noreply.github.com>

* Update sdk/storage/Azure.Storage.Blobs/AzureStorageNetMigrationV12.md

Co-authored-by: James <41338290+jaschrep-msft@users.noreply.github.com>

* Applying additions and changes from PR comments

* fixed hyperlinks

Co-authored-by: Rishab Pohane <ripohane@microsoft.com>
Co-authored-by: James <41338290+jaschrep-msft@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Storage Storage Service (Queues, Blobs, Files)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants