Azure Storage Turn on Fixed Tests #4422
Conversation
* SAS implementation * Fixed some minor formatting issues * Fixed checkstyle problems and test issue
alzimmermsft
changed the title
alzimmermsft
added
Client
Removes RawClients from Storage Blobs
…to ContainerClient (Azure#4376) * Removed raw clients * Added deleteContainer to StorageClient * Added getAppendBlob with snapshot to ContainerClient
* Initial check in for storage queue
| @@ -336,7 +337,7 @@ | |||
| * @return | |||
| * A reactive response containing the information of the cleared pages. | |||
| return null | ||
| } | ||
| @Override | ||
| Mono<String> bodyAsString(Charset charset) { |
There was a problem hiding this comment.
Is this necessary to have, it looks like a duplicate to the one above? Since it is a mock response, we can only include the necessary part.
There was a problem hiding this comment.
You have to include everything as it's implementing an interface, I should change extends -> implements
| @@ -15,8 +16,9 @@ | |||
|
|
|||
| @BeforeClass | |||
| public static void setup() { | |||
| String endpoint = String.format("https://%s.blob.core.windows.net", ConfigurationManager.getConfiguration().get("PRIMARY_STORAGE_ACCOUNT_KEY")); | |||
There was a problem hiding this comment.
PRIMARY_STORAGE_ACCOUNT_KEY. It should be account name
There was a problem hiding this comment.
This class is being deleted in another PR
| storageClient = new BlobServiceClientBuilder() | ||
| .endpoint("https://" + System.getenv("ACCOUNT_NAME") + ".blob.core.windows.net") | ||
| .endpoint(endpoint) | ||
| .credential(new EnvironmentCredential()) | ||
| // .httpClient(HttpClient.createDefault().proxy(() -> new ProxyOptions(Type.HTTP, new InetSocketAddress("localhost", 8888)))) |
There was a problem hiding this comment.
This class is being deleted in another PR
| @@ -57,6 +57,9 @@ class AppendBlobAPITest extends APISpec { | |||
| bu.create(headers, null, null, null) | |||
| Response<BlobProperties> response = bu.getProperties() | |||
|
|
|||
| // If the value isn't set the service will automatically set it | |||
| contentType = (contentType == null) ? "application/octet-stream" : contentType | |||
There was a problem hiding this comment.
What's the reason you remove it from APISpec but added here?
There was a problem hiding this comment.
This isn't true for all cases that used the method
| @@ -368,15 +372,15 @@ class BlobAPITest extends APISpec { | |||
| .blobContentType("type") | |||
| .blobCacheControl(properties.cacheControl()) | |||
| .blobContentLanguage(properties.contentLanguage()) | |||
| .blobContentMD5(Base64.getDecoder().decode(properties.contentMD5())) | |||
| .blobContentMD5(Base64.getEncoder().encode(MessageDigest.getInstance("MD5").digest(defaultData.array()))) | |||
There was a problem hiding this comment.
This line of changes is really different than original one. Why change it like this?
There was a problem hiding this comment.
The previous call was trying to decode the default data that was sent to the service incorrectly. This new call is just constructing the ContentMD5 from the actual source.
| }*/ | ||
| response.value().pageRange().size() == 1 | ||
| validateBasicHeaders(response.headers()) | ||
| Integer.parseInt(response.headers().value("x-ms-blob-content-length")) == PageBlobClient.PAGE_BYTES |
There was a problem hiding this comment.
Just curious, do we maintain the header value as "x-ms-blob-content-length" instead of "Content-Length"?
There was a problem hiding this comment.
This is a custom header that contains the blob size, Content-Length contains other information
|
Changes made in this PR were captured in #4491 |
Fixes #4325