[Storage] [STG105] Data Locality - #5273
vincenttran-msft wants to merge 6 commits into
Conversation
|
Azure Pipelines: Successfully started running 1 pipeline(s). 3 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
| } | ||
| } | ||
|
|
||
| #[recorded::test(live)] |
There was a problem hiding this comment.
This is live-only as when we add in the necessary fixtures to get it to play nicely with test proxy, it no longer did any re-routing. Probably having to do with some interference with how test proxy works.
There is also some temporary prints in there that I will remove- but we need to decide how we are going to move forward wrt test coverage of this feature.
| // This is a characterization test for the current emitter bug: 204 is an accepted | ||
| // success response, but the generated pager attempts to deserialize its empty body as XML. | ||
| // Once the emitter is fixed, this test should be changed to assert the corrected empty or | ||
| // optional result generated for a successful 204 response. | ||
| #[tokio::test] | ||
| async fn generated_list_layout_fails_to_deserialize_valid_no_content_response() { | ||
| let mock: Arc<dyn HttpClient> = Arc::new(MockHttpClient::new(|_req| { | ||
| async move { | ||
| Ok(AsyncRawResponse::from_bytes( | ||
| StatusCode::NoContent, | ||
| Headers::new(), | ||
| Bytes::new(), | ||
| )) | ||
| } | ||
| .boxed() | ||
| })); | ||
|
|
||
| let client = layout_client(mock); | ||
| let mut pages = client.list_layout(None).unwrap(); | ||
|
|
||
| let error = pages | ||
| .next() | ||
| .await | ||
| .expect("the service returned one response") | ||
| .expect_err("HTTP 204 should expose the generated XML deserialization bug"); | ||
|
|
||
| assert_eq!(*error.kind(), ErrorKind::DataConversion); | ||
| assert!(error | ||
| .to_string() | ||
| .contains("failed to deserialize the following xml")); | ||
| } |
There was a problem hiding this comment.
204: No Content gets attempted to be deserialized and so mistakenly bubbles up as a DataConversion, when in fact a 204 should be a successful response.
| /// The rewrite is computed on a scratch copy and only committed once fully | ||
| /// successful, so a malformed endpoint returns an error without leaving the | ||
| /// request in a half-rewritten state. | ||
| fn apply_layout_endpoint(request: &mut Request, endpoint: &str) -> azure_core::Result<()> { |
There was a problem hiding this comment.
We need to verify all this parsing works with custom endpoints as well.
There was a problem hiding this comment.
Added testcases for custom endpoints + Azurite-style endpoints (unit and mock). I agree that more testing in this area is probably desired, so will leave this open for discussion.
| if let Some(range) = &range { | ||
| if let Some(layout) = cache.current(&self.client).await { | ||
| if let Some(endpoint) = layout.ideal_endpoint(range.start as i64) { | ||
| opt.method_options.context = opt |
There was a problem hiding this comment.
We should write a test where a customer calls GetLayout themselves and sets this context themselves and make sure it actually uses that endpoint.
There was a problem hiding this comment.
Added test_download_layout_endpoint_selected_by_caller() to show this behavior.
…ose layout_endpoint to manually override, add testcases, unify to singular test file
2 participants
STG105 Feature - Data Locality
.tsp:https://github.com/Azure/azure-rest-api-specs/tree/rust/feature/storage_stg105