From a0e379387ce5044e36eeb6435db7b53a20ad82f7 Mon Sep 17 00:00:00 2001 From: Scott Davidson Date: Wed, 14 Jun 2023 16:28:56 +0100 Subject: [PATCH 1/9] Add explanatory comments for clarity --- src/operations.rs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/operations.rs b/src/operations.rs index a8538b3..8a1d833 100644 --- a/src/operations.rs +++ b/src/operations.rs @@ -174,13 +174,14 @@ mod tests { order: None, selection: None, }; - let data = [1, 2, 3, 4, 5, 6, 7, 8]; + let data: [u8; 8] = [1, 2, 3, 4, 5, 6, 7, 8]; let bytes = Bytes::copy_from_slice(&data); let response = Count::execute(&request_data, &bytes).unwrap(); + // A u8 slice of 8 elements == a u32 slice with 2 elements // Count is always i64. let expected: i64 = 2; assert_eq!(expected.as_bytes(), response.body); - assert_eq!(8, response.body.len()); + assert_eq!(8, response.body.len()); // Assert that count value is 8 bytes (i.e. i64) assert_eq!(models::DType::Int64, response.dtype); assert_eq!(vec![0; 0], response.shape); } @@ -198,7 +199,12 @@ mod tests { order: None, selection: None, }; - let data = [1, 2, 3, 4, 5, 6, 7, 8]; + // data: + // A u8 slice of 8 elements == a single i64 value + // where each slice element is 2 hexadecimal digits + // and the order is reversed on little-endian systems + // so [1, 2, 3] is 0x030201 as an i64 in hexadecimal + let data: [u8; 8] = [1, 2, 3, 4, 5, 6, 7, 8]; let bytes = Bytes::copy_from_slice(&data); let response = Max::execute(&request_data, &bytes).unwrap(); let expected: i64 = 0x0807060504030201; From fb5e8fec2bdcf8a10c97feddee2aa8868a9d8960 Mon Sep 17 00:00:00 2001 From: Scott Davidson Date: Wed, 14 Jun 2023 16:29:28 +0100 Subject: [PATCH 2/9] Fix version mismatch --- Cargo.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index 0347172..0f2105d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1578,7 +1578,7 @@ checksum = "f91339c0467de62360649f8d3e185ca8de4224ff281f66000de5eb2a77a79041" [[package]] name = "s3-active-storage" -version = "0.1.0" +version = "0.1.1" dependencies = [ "async-trait", "aws-credential-types", From 966bfb320d90d6150a30da0fc9b56fae39bc6dad Mon Sep 17 00:00:00 2001 From: Scott Davidson Date: Wed, 14 Jun 2023 16:30:02 +0100 Subject: [PATCH 3/9] Ignore scripts venv --- .gitignore | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.gitignore b/.gitignore index e2e3e66..febd41a 100644 --- a/.gitignore +++ b/.gitignore @@ -16,3 +16,6 @@ # Dev TLS certs .certs/ + +# Others +scripts/venv \ No newline at end of file From 54158adc89d6fe4ef03432f0c0489e0af1a332eb Mon Sep 17 00:00:00 2001 From: Scott Davidson Date: Wed, 14 Jun 2023 16:54:08 +0100 Subject: [PATCH 4/9] Add show-response-headers option --- scripts/client.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/scripts/client.py b/scripts/client.py index e4ac866..1d60999 100644 --- a/scripts/client.py +++ b/scripts/client.py @@ -36,6 +36,7 @@ def get_args() -> argparse.Namespace: parser.add_argument("--shape", type=str) parser.add_argument("--order", default="C") #, choices=["C", "F"]) allow invalid for testing parser.add_argument("--selection", type=str) + parser.add_argument("--show-response-headers", action=argparse.BooleanOptionalAction) return parser.parse_args() @@ -65,13 +66,17 @@ def request(url: str, username: str, password: str, request_data: dict): return response -def display(response): +def display(response, show_headers=False): #print(response.content) dtype = response.headers['x-activestorage-dtype'] shape = json.loads(response.headers['x-activestorage-shape']) result = np.frombuffer(response.content, dtype=dtype) result = result.reshape(shape) - print(result) + if show_headers: + print("\nResponse headers:", response.headers) + print("\nResult:", result) + else: + print(result) def display_error(response): @@ -88,7 +93,7 @@ def main(): url = f'{args.server}/v1/{args.operation}/' response = request(url, args.username, args.password, request_data) if response.ok: - display(response) + display(response, show_headers=args.show_response_headers) else: display_error(response) sys.exit(1) From a5ab9fa1dbf7d960f8f379faef3725212a9d180e Mon Sep 17 00:00:00 2001 From: Scott Davidson Date: Wed, 14 Jun 2023 16:55:09 +0100 Subject: [PATCH 5/9] Return count as response header --- src/app.rs | 3 ++ src/models.rs | 11 +++++-- src/operation.rs | 4 +++ src/operations.rs | 78 +++++++++++++++++++++++++++++++++++++++++++---- 4 files changed, 88 insertions(+), 8 deletions(-) diff --git a/src/app.rs b/src/app.rs index e4f9d14..d8caf9e 100644 --- a/src/app.rs +++ b/src/app.rs @@ -29,6 +29,8 @@ use tower_http::validate_request::ValidateRequestHeaderLayer; static HEADER_DTYPE: header::HeaderName = header::HeaderName::from_static("x-activestorage-dtype"); /// `x-activestorage-shape` header definition static HEADER_SHAPE: header::HeaderName = header::HeaderName::from_static("x-activestorage-shape"); +/// `x-activestorage-count` header definition +static HEADER_COUNT: header::HeaderName = header::HeaderName::from_static("x-activestorage-count"); impl IntoResponse for models::Response { /// Convert a [crate::models::Response] into a [axum::response::Response]. @@ -41,6 +43,7 @@ impl IntoResponse for models::Response { ), (&HEADER_DTYPE, self.dtype.to_string().to_lowercase()), (&HEADER_SHAPE, serde_json::to_string(&self.shape).unwrap()), + (&HEADER_COUNT, serde_json::to_string(&self.count).unwrap()), ], self.body, ) diff --git a/src/models.rs b/src/models.rs index 5dc2c86..088419d 100644 --- a/src/models.rs +++ b/src/models.rs @@ -186,12 +186,19 @@ pub struct Response { pub dtype: DType, /// Shape of the response pub shape: Vec, + /// Number of non-missing elements operated on to generate response + pub count: i64, } impl Response { /// Return a Response object - pub fn new(body: Bytes, dtype: DType, shape: Vec) -> Response { - Response { body, dtype, shape } + pub fn new(body: Bytes, dtype: DType, shape: Vec, count: i64) -> Response { + Response { + body, + dtype, + shape, + count, + } } } diff --git a/src/operation.rs b/src/operation.rs index 2fab4d5..36746f8 100644 --- a/src/operation.rs +++ b/src/operation.rs @@ -102,6 +102,7 @@ mod tests { data.clone(), request_data.dtype, vec![3], + 3, )) } } @@ -125,6 +126,7 @@ mod tests { assert_eq!(&[1, 2, 3, 4][..], response.body); assert_eq!(models::DType::Uint32, response.dtype); assert_eq!(vec![3], response.shape); + assert_eq!(3, response.count); } struct TestNumOp {} @@ -140,6 +142,7 @@ mod tests { body.into(), request_data.dtype, vec![1, 2], + 2, )) } } @@ -163,5 +166,6 @@ mod tests { assert_eq!("i64", response.body); assert_eq!(models::DType::Int64, response.dtype); assert_eq!(vec![1, 2], response.shape); + assert_eq!(2, response.count); } } diff --git a/src/operations.rs b/src/operations.rs index 8a1d833..5dc8f59 100644 --- a/src/operations.rs +++ b/src/operations.rs @@ -10,9 +10,12 @@ use crate::operation::{Element, NumOperation}; use axum::body::Bytes; use ndarray_stats::{errors::MinMaxError, QuantileExt}; +use num_traits::ToPrimitive; // Bring trait into scope to use as_bytes method. use zerocopy::AsBytes; +// TODO: Remove count method from API once count-as-response-header is implemented + /// Return the number of selected elements in the array. pub struct Count {} @@ -29,7 +32,12 @@ impl NumOperation for Count { let body = len.to_le_bytes(); // Need to copy to provide ownership to caller. let body = Bytes::copy_from_slice(&body); - Ok(models::Response::new(body, models::DType::Int64, vec![])) + Ok(models::Response::new( + body, + models::DType::Int64, + vec![], + len, + )) } } @@ -44,6 +52,11 @@ impl NumOperation for Max { let array = array::build_array::(request_data, data)?; let slice_info = array::build_slice_info::(&request_data.selection, array.shape()); let sliced = array.slice(slice_info); + // FIXME: Account for missing data? + let count = sliced + .len() + .to_i64() + .expect("Count to be convertible to i64"); // FIXME: endianness? let body = sliced .max() @@ -54,7 +67,12 @@ impl NumOperation for Max { .as_bytes(); // Need to copy to provide ownership to caller. let body = Bytes::copy_from_slice(body); - Ok(models::Response::new(body, request_data.dtype, vec![])) + Ok(models::Response::new( + body, + request_data.dtype, + vec![], + count, + )) } } @@ -69,6 +87,11 @@ impl NumOperation for Mean { let array = array::build_array::(request_data, data)?; let slice_info = array::build_slice_info::(&request_data.selection, array.shape()); let sliced = array.slice(slice_info); + // FIXME: Account for missing data? + let count = sliced + .len() + .to_i64() + .expect("Count to be convertible to i64"); // FIXME: endianness? let body = sliced .mean() @@ -76,7 +99,12 @@ impl NumOperation for Mean { let body = body.as_bytes(); // Need to copy to provide ownership to caller. let body = Bytes::copy_from_slice(body); - Ok(models::Response::new(body, request_data.dtype, vec![])) + Ok(models::Response::new( + body, + request_data.dtype, + vec![], + count, + )) } } @@ -91,6 +119,11 @@ impl NumOperation for Min { let array = array::build_array::(request_data, data)?; let slice_info = array::build_slice_info::(&request_data.selection, array.shape()); let sliced = array.slice(slice_info); + // FIXME: Account for missing data? + let count = sliced + .len() + .to_i64() + .expect("Count to be convertible to i64"); // FIXME: endianness? let body = sliced .min() @@ -101,7 +134,12 @@ impl NumOperation for Min { .as_bytes(); // Need to copy to provide ownership to caller. let body = Bytes::copy_from_slice(body); - Ok(models::Response::new(body, request_data.dtype, vec![])) + Ok(models::Response::new( + body, + request_data.dtype, + vec![], + count, + )) } } @@ -116,6 +154,11 @@ impl NumOperation for Select { let array = array::build_array::(request_data, data)?; let slice_info = array::build_slice_info::(&request_data.selection, array.shape()); let sliced = array.slice(slice_info); + // FIXME: Account for missing data? + let count = sliced + .len() + .to_i64() + .expect("Count to be convertible to i64"); let shape = sliced.shape().to_vec(); // Transpose Fortran ordered arrays before iterating. let body = if !array.is_standard_layout() { @@ -129,7 +172,12 @@ impl NumOperation for Select { let body = body.as_bytes(); // Need to copy to provide ownership to caller. let body = Bytes::copy_from_slice(body); - Ok(models::Response::new(body, request_data.dtype, shape)) + Ok(models::Response::new( + body, + request_data.dtype, + shape, + count, + )) } } @@ -144,12 +192,22 @@ impl NumOperation for Sum { let array = array::build_array::(request_data, data)?; let slice_info = array::build_slice_info::(&request_data.selection, array.shape()); let sliced = array.slice(slice_info); + // FIXME: Account for missing data? + let count = sliced + .len() + .to_i64() + .expect("Count to be convertible to i64"); // FIXME: endianness? let body = sliced.sum(); let body = body.as_bytes(); // Need to copy to provide ownership to caller. let body = Bytes::copy_from_slice(body); - Ok(models::Response::new(body, request_data.dtype, vec![])) + Ok(models::Response::new( + body, + request_data.dtype, + vec![], + count, + )) } } @@ -184,6 +242,7 @@ mod tests { assert_eq!(8, response.body.len()); // Assert that count value is 8 bytes (i.e. i64) assert_eq!(models::DType::Int64, response.dtype); assert_eq!(vec![0; 0], response.shape); + assert_eq!(expected, response.count); } #[test] @@ -212,6 +271,7 @@ mod tests { assert_eq!(8, response.body.len()); assert_eq!(models::DType::Int64, response.dtype); assert_eq!(vec![0; 0], response.shape); + assert_eq!(1, response.count); } #[test] @@ -235,6 +295,7 @@ mod tests { assert_eq!(4, response.body.len()); assert_eq!(models::DType::Uint32, response.dtype); assert_eq!(vec![0; 0], response.shape); + assert_eq!(2, response.count); } #[test] @@ -258,6 +319,7 @@ mod tests { assert_eq!(8, response.body.len()); assert_eq!(models::DType::Uint64, response.dtype); assert_eq!(vec![0; 0], response.shape); + assert_eq!(1, response.count); } #[test] @@ -281,6 +343,7 @@ mod tests { assert_eq!(8, response.body.len()); assert_eq!(models::DType::Float32, response.dtype); assert_eq!(vec![2], response.shape); + assert_eq!(2, response.count); } #[test] @@ -304,6 +367,7 @@ mod tests { assert_eq!(16, response.body.len()); assert_eq!(models::DType::Float64, response.dtype); assert_eq!(vec![2, 1], response.shape); + assert_eq!(2, response.count); } #[test] @@ -333,6 +397,7 @@ mod tests { assert_eq!(8, response.body.len()); assert_eq!(models::DType::Float32, response.dtype); assert_eq!(vec![2, 1], response.shape); + assert_eq!(2, response.count); } #[test] @@ -356,5 +421,6 @@ mod tests { assert_eq!(4, response.body.len()); assert_eq!(models::DType::Uint32, response.dtype); assert_eq!(vec![0; 0], response.shape); + assert_eq!(2, response.count); } } From e2838d1cc69eac55ea25b15cdea7dbd00aa7a9c6 Mon Sep 17 00:00:00 2001 From: Scott Davidson Date: Wed, 14 Jun 2023 16:57:31 +0100 Subject: [PATCH 6/9] Use existing ActiveStorageError handling for count header --- src/operations.rs | 26 +++++--------------------- 1 file changed, 5 insertions(+), 21 deletions(-) diff --git a/src/operations.rs b/src/operations.rs index 5dc8f59..ea96577 100644 --- a/src/operations.rs +++ b/src/operations.rs @@ -10,7 +10,6 @@ use crate::operation::{Element, NumOperation}; use axum::body::Bytes; use ndarray_stats::{errors::MinMaxError, QuantileExt}; -use num_traits::ToPrimitive; // Bring trait into scope to use as_bytes method. use zerocopy::AsBytes; @@ -53,10 +52,7 @@ impl NumOperation for Max { let slice_info = array::build_slice_info::(&request_data.selection, array.shape()); let sliced = array.slice(slice_info); // FIXME: Account for missing data? - let count = sliced - .len() - .to_i64() - .expect("Count to be convertible to i64"); + let count = i64::try_from(sliced.len())?; // FIXME: endianness? let body = sliced .max() @@ -88,10 +84,7 @@ impl NumOperation for Mean { let slice_info = array::build_slice_info::(&request_data.selection, array.shape()); let sliced = array.slice(slice_info); // FIXME: Account for missing data? - let count = sliced - .len() - .to_i64() - .expect("Count to be convertible to i64"); + let count = i64::try_from(sliced.len())?; // FIXME: endianness? let body = sliced .mean() @@ -120,10 +113,7 @@ impl NumOperation for Min { let slice_info = array::build_slice_info::(&request_data.selection, array.shape()); let sliced = array.slice(slice_info); // FIXME: Account for missing data? - let count = sliced - .len() - .to_i64() - .expect("Count to be convertible to i64"); + let count = i64::try_from(sliced.len())?; // FIXME: endianness? let body = sliced .min() @@ -155,10 +145,7 @@ impl NumOperation for Select { let slice_info = array::build_slice_info::(&request_data.selection, array.shape()); let sliced = array.slice(slice_info); // FIXME: Account for missing data? - let count = sliced - .len() - .to_i64() - .expect("Count to be convertible to i64"); + let count = i64::try_from(sliced.len())?; let shape = sliced.shape().to_vec(); // Transpose Fortran ordered arrays before iterating. let body = if !array.is_standard_layout() { @@ -193,10 +180,7 @@ impl NumOperation for Sum { let slice_info = array::build_slice_info::(&request_data.selection, array.shape()); let sliced = array.slice(slice_info); // FIXME: Account for missing data? - let count = sliced - .len() - .to_i64() - .expect("Count to be convertible to i64"); + let count = i64::try_from(sliced.len())?; // FIXME: endianness? let body = sliced.sum(); let body = body.as_bytes(); From a227af86efd7683a0ace6670471b6265d5083e6e Mon Sep 17 00:00:00 2001 From: Scott Davidson Date: Wed, 14 Jun 2023 17:06:29 +0100 Subject: [PATCH 7/9] Ignore .vscode --- .gitignore | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index febd41a..398e883 100644 --- a/.gitignore +++ b/.gitignore @@ -18,4 +18,5 @@ .certs/ # Others -scripts/venv \ No newline at end of file +scripts/venv +.vscode \ No newline at end of file From c69ea85e888f01564c44715f76294eb05f75119b Mon Sep 17 00:00:00 2001 From: Scott Davidson Date: Wed, 5 Jul 2023 10:27:45 +0100 Subject: [PATCH 8/9] Mention `x-activestorage-count` header --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 9ff4dd0..40514c6 100644 --- a/README.md +++ b/README.md @@ -79,7 +79,7 @@ with a JSON payload of the form: The currently supported reducers are `max`, `min`, `mean`, `sum`, `select` and `count`. All reducers return the result using the same datatype as specified in the request except for `count` which always returns the result as `int64`. -The proxy adds two custom headers `x-activestorage-dtype` and `x-activestrorage-shape` to the HTTP response to allow the numeric result to be reconstructed from the binary content of the response. +The proxy adds two custom headers `x-activestorage-dtype` and `x-activestrorage-shape` to the HTTP response to allow the numeric result to be reconstructed from the binary content of the response. An additional `x-activestorage-count` header is also returned which contains the number of array elements operated on while performing the requested reduction. This header is useful, for example, to calculate the mean over multiple requests where the number of items operated on may differ between chunks. [//]: <> (TODO: No OpenAPI support yet). [//]: <> (For a running instance of the proxy server, the full OpenAPI specification is browsable as a web page at the `{proxy-address}/redoc/` endpoint or in raw JSON form at `{proxy-address}/openapi.json`.) From 3423c33f6c989c5aa0bd5dddb1f3ab4c648aac1d Mon Sep 17 00:00:00 2001 From: Scott Davidson Date: Wed, 5 Jul 2023 10:28:16 +0100 Subject: [PATCH 9/9] Remove obsolete comment --- src/operations.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/operations.rs b/src/operations.rs index ea96577..730ffb3 100644 --- a/src/operations.rs +++ b/src/operations.rs @@ -13,8 +13,6 @@ use ndarray_stats::{errors::MinMaxError, QuantileExt}; // Bring trait into scope to use as_bytes method. use zerocopy::AsBytes; -// TODO: Remove count method from API once count-as-response-header is implemented - /// Return the number of selected elements in the array. pub struct Count {}