From 0b37608d4f39d63fc059c9fa9a0648b1c5e9dc46 Mon Sep 17 00:00:00 2001 From: Shreya Shankar Date: Fri, 18 Aug 2023 00:00:10 -0700 Subject: [PATCH 01/37] Working? version of rusty state --- .gitignore | 2 +- rustystate/Cargo.toml | 10 ++ rustystate/pyproject.toml | 9 ++ rustystate/src/lib.rs | 199 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 219 insertions(+), 1 deletion(-) create mode 100644 rustystate/Cargo.toml create mode 100644 rustystate/pyproject.toml create mode 100644 rustystate/src/lib.rs diff --git a/.gitignore b/.gitignore index c18b5719..ca45f5d2 100644 --- a/.gitignore +++ b/.gitignore @@ -18,4 +18,4 @@ site* projects unnecessary.py motionstate* -*.whl \ No newline at end of file +*.whl diff --git a/rustystate/Cargo.toml b/rustystate/Cargo.toml new file mode 100644 index 00000000..ca0ed096 --- /dev/null +++ b/rustystate/Cargo.toml @@ -0,0 +1,10 @@ +[package] +name = "rustystate" +version = "0.1.0" +edition = "2021" + +# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html + +[dependencies] +redis = "0.19" +pyo3 = { version = "0.15", features = ["extension-module"] } diff --git a/rustystate/pyproject.toml b/rustystate/pyproject.toml new file mode 100644 index 00000000..7cc6ceaa --- /dev/null +++ b/rustystate/pyproject.toml @@ -0,0 +1,9 @@ +[build-system] +requires = ["maturin"] +build-backend = "maturin" + +[project] +name = "rustystate" +version = "0.1.0" +authors = [{name = "Shreya Shankar", email = "ss.shankar505@gmail.com"}] +description = "A rust-state Python extension" diff --git a/rustystate/src/lib.rs b/rustystate/src/lib.rs new file mode 100644 index 00000000..69887154 --- /dev/null +++ b/rustystate/src/lib.rs @@ -0,0 +1,199 @@ +use pyo3::exceptions; +use pyo3::prelude::*; +use pyo3::types::{PyAny, PyBytes, PyDict, PyFloat, PyInt, PyList, PyString}; +use redis::Commands; +use std::collections::HashMap; + +/* +TODO: +* Increment version when calling set_bulk +* Remove set method (unnecessary) + */ + +#[pyclass] +pub struct State { + component_name: String, + instance_id: String, + client: redis::Client, + cache: HashMap>, +} + +#[pymethods] +impl State { + #[new] + pub fn new(component_name: String, instance_id: String, redis_url: &str) -> PyResult { + let client = redis::Client::open(redis_url).map_err(|err| { + PyErr::new::(format!( + "Redis connection error: {}", + err + )) + })?; + Ok(State { + component_name, + instance_id, + client, + cache: HashMap::new(), + }) + } + + pub fn set(&mut self, py: Python, key: String, value: &PyAny) -> PyResult<()> { + let mut con = self.client.get_connection().unwrap(); + + let serialized_data = serialize_value(py, value)?; + + self.cache.insert(key.clone(), serialized_data.clone()); + con.set::<_, _, ()>(key, serialized_data).unwrap(); + Ok(()) + } + + pub fn bulk_set(&mut self, py: Python, items: &PyDict) -> PyResult<()> { + let mut con = self.client.get_connection().unwrap(); + let mut pipeline = redis::pipe(); + + // Iterate over the items in the dictionary + for (key, value) in items { + let key_str = key.extract::()?; + let serialized_data = serialize_value(py, value)?; + + // Insert the key and value into the cache + self.cache.insert(key_str.clone(), serialized_data.clone()); + // Insert the key and value into the pipeline + //pipeline.set::<_, _, ()>(key_str, serialized_data); + pipeline.cmd("SET").arg(key_str).arg(serialized_data); + } + + // Execute the pipeline, throwing a Python error if it fails + pipeline.query::<()>(&mut con).map_err(|err| { + PyErr::new::(format!( + "Redis bulk set error: {}", + err + )) + })?; + + Ok(()) + } + + pub fn get(&mut self, py: Python, key: &str) -> PyResult { + // If the key is in the cache, return it + if let Some(value) = self.cache.get(key) { + return deserialize_value(py, value); + } + + // Otherwise, fetch it from Redis + let mut con = self.client.get_connection().unwrap(); + let result_data: redis::RedisResult>> = con.get(key); + + match result_data { + Ok(Some(data)) => { + // Insert the key and value into the cache + self.cache.insert(key.to_string(), data.clone()); + // Deserialize the value + deserialize_value(py, &data) + } + Ok(None) => Err(PyErr::new::("Key not found")), + Err(err) => Err(PyErr::new::(format!( + "Redis get error: {}", + err + ))), + } + } +} + +// Serialization Helpers + +fn serialize_list(py: Python, value: &PyAny) -> PyResult>> { + let list = value.downcast::()?; + let mut serialized_items = Vec::new(); + + for item in list.iter() { + if item.is_instance::()? { + serialized_items.push(item.str()?.to_string()); + } else if item.is_instance::()? { + serialized_items.push(item.str()?.to_string()); + } else if item.is_instance::()? { + serialized_items.push(item.str()?.to_string()); + } else { + // If the list contains non-primitive types, return None + return Ok(None); + } + } + + // Joining the serialized items with a delimiter (e.g., `|`), you can choose another delimiter if you wish. + Ok(Some(serialized_items.join("|").into_bytes())) +} + +fn serialize_value(py: Python, value: &PyAny) -> PyResult> { + if value.is_instance::()? + || value.is_instance::()? + || value.is_instance::()? + { + Ok(value.str()?.to_string().into_bytes()) + } else if value.is_instance::()? { + if let Some(serialized) = serialize_list(py, value)? { + Ok(serialized) + } else { + // If couldn't serialize as a list of primitives, use cloudpickle + let cloudpickle = py.import("cloudpickle")?; + let bytes = cloudpickle + .getattr("dumps")? + .call1((value,))? + .extract::<&PyBytes>()?; + Ok(bytes.as_bytes().to_vec()) + } + } else { + // Use cloudpickle for other types + let cloudpickle = py.import("cloudpickle")?; + let bytes = cloudpickle + .getattr("dumps")? + .call1((value,))? + .extract::<&PyBytes>()?; + Ok(bytes.as_bytes().to_vec()) + } +} + +fn deserialize_value(py: Python, value: &[u8]) -> PyResult { + if let Ok(decoded) = std::str::from_utf8(value) { + if let Ok(int_value) = decoded.parse::() { + Ok(int_value.into_py(py)) + } else if let Ok(float_value) = decoded.parse::() { + Ok(float_value.into_py(py)) + } else if decoded.contains("|") { + // Detecting our delimiter to identify lists + let items: Vec<_> = decoded + .split('|') + .map(|item| { + if let Ok(int_value) = item.parse::() { + int_value.into_py(py) + } else if let Ok(float_value) = item.parse::() { + float_value.into_py(py) + } else { + item.to_string().into_py(py) + } + }) + .collect(); + Ok(PyList::new(py, &items).into()) + } else { + Ok(decoded.to_string().into_py(py)) + } + } else { + let cloudpickle = py.import("cloudpickle")?; + let bytes_value = PyBytes::new(py, value); + let obj = cloudpickle.getattr("loads")?.call1((bytes_value,))?; + Ok(obj.into()) + } +} + +#[pymodule] +fn rustystate(_py: Python, m: &PyModule) -> PyResult<()> { + m.add_class::()?; + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + #[test] + fn it_works() { + assert_eq!(2 + 2, 4); + } +} From c35e5592a85d3ffe435d21f61e3748c9db5398ec Mon Sep 17 00:00:00 2001 From: Shreya Shankar Date: Fri, 18 Aug 2023 00:16:03 -0700 Subject: [PATCH 02/37] Working version with lists and dicts --- rustystate/src/lib.rs | 241 +++++++++++++++++++++++++++++++----------- 1 file changed, 179 insertions(+), 62 deletions(-) diff --git a/rustystate/src/lib.rs b/rustystate/src/lib.rs index 69887154..b2a2081b 100644 --- a/rustystate/src/lib.rs +++ b/rustystate/src/lib.rs @@ -100,26 +100,23 @@ impl State { } // Serialization Helpers +const MARKER_LIST: u8 = 0x01; +const MARKER_DICT: u8 = 0x02; -fn serialize_list(py: Python, value: &PyAny) -> PyResult>> { - let list = value.downcast::()?; - let mut serialized_items = Vec::new(); - - for item in list.iter() { - if item.is_instance::()? { - serialized_items.push(item.str()?.to_string()); - } else if item.is_instance::()? { - serialized_items.push(item.str()?.to_string()); - } else if item.is_instance::()? { - serialized_items.push(item.str()?.to_string()); - } else { - // If the list contains non-primitive types, return None - return Ok(None); - } - } +fn cloudpickle_serialize(py: Python, value: &PyAny) -> PyResult> { + let cloudpickle = py.import("cloudpickle")?; + let bytes = cloudpickle + .getattr("dumps")? + .call1((value,))? + .extract::<&PyBytes>()?; + Ok(bytes.as_bytes().to_vec()) +} - // Joining the serialized items with a delimiter (e.g., `|`), you can choose another delimiter if you wish. - Ok(Some(serialized_items.join("|").into_bytes())) +fn cloudpickle_deserialize(py: Python, value: &[u8]) -> PyResult { + let cloudpickle = py.import("cloudpickle")?; + let bytes_value = PyBytes::new(py, value); + let obj = cloudpickle.getattr("loads")?.call1((bytes_value,))?; + Ok(obj.into()) } fn serialize_value(py: Python, value: &PyAny) -> PyResult> { @@ -128,61 +125,181 @@ fn serialize_value(py: Python, value: &PyAny) -> PyResult> { || value.is_instance::()? { Ok(value.str()?.to_string().into_bytes()) + } else if value.is_instance::()? { + let mut serialized = vec![MARKER_DICT]; + serialized.extend(serialize_dict(py, value)?); + Ok(serialized) } else if value.is_instance::()? { - if let Some(serialized) = serialize_list(py, value)? { - Ok(serialized) - } else { - // If couldn't serialize as a list of primitives, use cloudpickle - let cloudpickle = py.import("cloudpickle")?; - let bytes = cloudpickle - .getattr("dumps")? - .call1((value,))? - .extract::<&PyBytes>()?; - Ok(bytes.as_bytes().to_vec()) + let list = value.downcast::()?; + let mut serialized = vec![MARKER_LIST]; + for item in list.iter() { + let serialized_item = serialize_value(py, item)?; + serialized.extend((serialized_item.len() as u64).to_le_bytes().iter()); + serialized.extend(serialized_item); } + Ok(serialized) } else { - // Use cloudpickle for other types - let cloudpickle = py.import("cloudpickle")?; - let bytes = cloudpickle - .getattr("dumps")? - .call1((value,))? - .extract::<&PyBytes>()?; - Ok(bytes.as_bytes().to_vec()) + cloudpickle_serialize(py, value) + } +} + +fn serialize_dict(py: Python, value: &PyAny) -> PyResult> { + let dict = value.downcast::()?; + let mut serialized = Vec::new(); + + for (key, val) in dict { + let key_bytes = serialize_value(py, key)?; + let val_bytes = serialize_value(py, val)?; + serialized.extend((key_bytes.len() as u64).to_le_bytes().iter()); + serialized.extend(key_bytes); + serialized.extend((val_bytes.len() as u64).to_le_bytes().iter()); + serialized.extend(val_bytes); } + + Ok(serialized) } fn deserialize_value(py: Python, value: &[u8]) -> PyResult { - if let Ok(decoded) = std::str::from_utf8(value) { - if let Ok(int_value) = decoded.parse::() { - Ok(int_value.into_py(py)) - } else if let Ok(float_value) = decoded.parse::() { - Ok(float_value.into_py(py)) - } else if decoded.contains("|") { - // Detecting our delimiter to identify lists - let items: Vec<_> = decoded - .split('|') - .map(|item| { - if let Ok(int_value) = item.parse::() { - int_value.into_py(py) - } else if let Ok(float_value) = item.parse::() { - float_value.into_py(py) - } else { - item.to_string().into_py(py) - } - }) - .collect(); - Ok(PyList::new(py, &items).into()) - } else { - Ok(decoded.to_string().into_py(py)) + if value.is_empty() { + return Err(PyErr::new::( + "Empty data", + )); + } + + // Check the marker first + match value[0] { + MARKER_LIST => { + let list = pyo3::types::PyList::empty(py); + let mut cursor = 1; + while cursor < value.len() { + let item_len = + u64::from_le_bytes(value[cursor..cursor + 8].try_into().unwrap()) as usize; + cursor += 8; + let item = deserialize_value(py, &value[cursor..cursor + item_len])?; + list.append(item)?; + cursor += item_len; + } + Ok(list.into()) + } + MARKER_DICT => { + let dict = PyDict::new(py); + let mut cursor = 1; + while cursor < value.len() { + let key_len = + u64::from_le_bytes(value[cursor..cursor + 8].try_into().unwrap()) as usize; + cursor += 8; + let key = deserialize_value(py, &value[cursor..cursor + key_len])?; + cursor += key_len; + + let val_len = + u64::from_le_bytes(value[cursor..cursor + 8].try_into().unwrap()) as usize; + cursor += 8; + let val = deserialize_value(py, &value[cursor..cursor + val_len])?; + cursor += val_len; + + dict.set_item(key, val)?; + } + Ok(dict.into()) + } + _ => { + // Only if it's not a marked value, we try to interpret it as a string + if let Ok(decoded) = std::str::from_utf8(value) { + if let Ok(int_value) = decoded.parse::() { + return Ok(int_value.into_py(py)); + } else if let Ok(float_value) = decoded.parse::() { + return Ok(float_value.into_py(py)); + } + return Ok(decoded.to_string().into_py(py)); + } + + // Default to cloudpickle deserialization for unrecognized patterns + cloudpickle_deserialize(py, value) } - } else { - let cloudpickle = py.import("cloudpickle")?; - let bytes_value = PyBytes::new(py, value); - let obj = cloudpickle.getattr("loads")?.call1((bytes_value,))?; - Ok(obj.into()) } } +// fn serialize_list(py: Python, value: &PyAny) -> PyResult>> { +// let list = value.downcast::()?; +// let mut serialized_items = Vec::new(); + +// for item in list.iter() { +// if item.is_instance::()? { +// serialized_items.push(item.str()?.to_string()); +// } else if item.is_instance::()? { +// serialized_items.push(item.str()?.to_string()); +// } else if item.is_instance::()? { +// serialized_items.push(item.str()?.to_string()); +// } else { +// // If the list contains non-primitive types, return None +// return Ok(None); +// } +// } + +// // Joining the serialized items with a delimiter (e.g., `|`), you can choose another delimiter if you wish. +// Ok(Some(serialized_items.join("|").into_bytes())) +// } + +// fn serialize_value(py: Python, value: &PyAny) -> PyResult> { +// if value.is_instance::()? +// || value.is_instance::()? +// || value.is_instance::()? +// { +// Ok(value.str()?.to_string().into_bytes()) +// } else if value.is_instance::()? { +// if let Some(serialized) = serialize_list(py, value)? { +// Ok(serialized) +// } else { +// // If couldn't serialize as a list of primitives, use cloudpickle +// let cloudpickle = py.import("cloudpickle")?; +// let bytes = cloudpickle +// .getattr("dumps")? +// .call1((value,))? +// .extract::<&PyBytes>()?; +// Ok(bytes.as_bytes().to_vec()) +// } +// } else { +// // Use cloudpickle for other types +// let cloudpickle = py.import("cloudpickle")?; +// let bytes = cloudpickle +// .getattr("dumps")? +// .call1((value,))? +// .extract::<&PyBytes>()?; +// Ok(bytes.as_bytes().to_vec()) +// } +// } + +// fn deserialize_value(py: Python, value: &[u8]) -> PyResult { +// if let Ok(decoded) = std::str::from_utf8(value) { +// if let Ok(int_value) = decoded.parse::() { +// Ok(int_value.into_py(py)) +// } else if let Ok(float_value) = decoded.parse::() { +// Ok(float_value.into_py(py)) +// } else if decoded.contains("|") { +// // Detecting our delimiter to identify lists +// let items: Vec<_> = decoded +// .split('|') +// .map(|item| { +// if let Ok(int_value) = item.parse::() { +// int_value.into_py(py) +// } else if let Ok(float_value) = item.parse::() { +// float_value.into_py(py) +// } else { +// item.to_string().into_py(py) +// } +// }) +// .collect(); +// Ok(PyList::new(py, &items).into()) +// } else { +// Ok(decoded.to_string().into_py(py)) +// } +// } else { +// let cloudpickle = py.import("cloudpickle")?; +// let bytes_value = PyBytes::new(py, value); +// let obj = cloudpickle.getattr("loads")?.call1((bytes_value,))?; +// Ok(obj.into()) +// } +// } + #[pymodule] fn rustystate(_py: Python, m: &PyModule) -> PyResult<()> { m.add_class::()?; From ad20cfb81013b2e6901c97cd2a78b2c6bc8327ce Mon Sep 17 00:00:00 2001 From: Shreya Shankar Date: Fri, 18 Aug 2023 00:23:44 -0700 Subject: [PATCH 03/37] Supposedly even more optimized rust --- rustystate/src/lib.rs | 56 +++++++++++++++++++++---------------------- 1 file changed, 28 insertions(+), 28 deletions(-) diff --git a/rustystate/src/lib.rs b/rustystate/src/lib.rs index b2a2081b..0c668d3f 100644 --- a/rustystate/src/lib.rs +++ b/rustystate/src/lib.rs @@ -159,6 +159,19 @@ fn serialize_dict(py: Python, value: &PyAny) -> PyResult> { Ok(serialized) } +fn extract_next_slice<'a>(value: &'a [u8], cursor: &mut usize) -> Option<&'a [u8]> { + if *cursor + 8 <= value.len() { + let len = u64::from_le_bytes(value[*cursor..*cursor + 8].try_into().unwrap()) as usize; + *cursor += 8; + if *cursor + len <= value.len() { + let result = &value[*cursor..*cursor + len]; + *cursor += len; + return Some(result); + } + } + None +} + fn deserialize_value(py: Python, value: &[u8]) -> PyResult { if value.is_empty() { return Err(PyErr::new::( @@ -166,54 +179,41 @@ fn deserialize_value(py: Python, value: &[u8]) -> PyResult { )); } - // Check the marker first - match value[0] { + let mut cursor = 0; + match value[cursor] { MARKER_LIST => { + cursor += 1; let list = pyo3::types::PyList::empty(py); - let mut cursor = 1; - while cursor < value.len() { - let item_len = - u64::from_le_bytes(value[cursor..cursor + 8].try_into().unwrap()) as usize; - cursor += 8; - let item = deserialize_value(py, &value[cursor..cursor + item_len])?; + while let Some(item_bytes) = extract_next_slice(value, &mut cursor) { + let item = deserialize_value(py, item_bytes)?; list.append(item)?; - cursor += item_len; } Ok(list.into()) } MARKER_DICT => { + cursor += 1; let dict = PyDict::new(py); - let mut cursor = 1; - while cursor < value.len() { - let key_len = - u64::from_le_bytes(value[cursor..cursor + 8].try_into().unwrap()) as usize; - cursor += 8; - let key = deserialize_value(py, &value[cursor..cursor + key_len])?; - cursor += key_len; - - let val_len = - u64::from_le_bytes(value[cursor..cursor + 8].try_into().unwrap()) as usize; - cursor += 8; - let val = deserialize_value(py, &value[cursor..cursor + val_len])?; - cursor += val_len; - + while let (Some(key_bytes), Some(val_bytes)) = ( + extract_next_slice(value, &mut cursor), + extract_next_slice(value, &mut cursor), + ) { + let key = deserialize_value(py, key_bytes)?; + let val = deserialize_value(py, val_bytes)?; dict.set_item(key, val)?; } Ok(dict.into()) } _ => { - // Only if it's not a marked value, we try to interpret it as a string if let Ok(decoded) = std::str::from_utf8(value) { if let Ok(int_value) = decoded.parse::() { return Ok(int_value.into_py(py)); } else if let Ok(float_value) = decoded.parse::() { return Ok(float_value.into_py(py)); } - return Ok(decoded.to_string().into_py(py)); + Ok(decoded.to_string().into_py(py)) + } else { + cloudpickle_deserialize(py, value) } - - // Default to cloudpickle deserialization for unrecognized patterns - cloudpickle_deserialize(py, value) } } } From fc1831e9b638d157d3bd3a53ae31bb8a0666cee4 Mon Sep 17 00:00:00 2001 From: Shreya Shankar Date: Fri, 18 Aug 2023 02:29:01 -0700 Subject: [PATCH 04/37] Successfully imported bare bones library version --- .gitignore | 9 ++ Makefile | 16 ++- motion/__init__.py | 3 + {rustystate => motionstate}/Cargo.toml | 9 +- {rustystate => motionstate}/src/lib.rs | 187 ++++++++++++------------- motionstate/src/state_value.rs | 41 ++++++ rustystate/pyproject.toml | 9 -- 7 files changed, 165 insertions(+), 109 deletions(-) rename {rustystate => motionstate}/Cargo.toml (55%) rename {rustystate => motionstate}/src/lib.rs (64%) create mode 100644 motionstate/src/state_value.rs delete mode 100644 rustystate/pyproject.toml diff --git a/.gitignore b/.gitignore index ca45f5d2..b95a71e8 100644 --- a/.gitignore +++ b/.gitignore @@ -16,6 +16,15 @@ dist* site* *package-lock.json projects +<<<<<<< HEAD unnecessary.py motionstate* *.whl +======= + +motionstate/debug* +motionstate/target* +motionstate/Cargo.lock +*rustc* +motionstate*.so +>>>>>>> e4aede0 (Successfully imported bare bones library version) diff --git a/Makefile b/Makefile index 5588a9b3..1a58c843 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,4 @@ -.PHONY: tests lint install mypy update docs +.PHONY: tests lint install mypy update docs statebuild tests: poetry run pytest @@ -17,4 +17,16 @@ update: poetry update docs: - poetry run mkdocs serve \ No newline at end of file + poetry run mkdocs serve + +statebuild: + cd motionstate && \ + echo "Building motionstate" && \ + maturin build --release && \ + python -m venv ../.motionenv && \ + source ../.motionenv/bin/activate && pip install target/wheels/motionstate*.whl && deactivate && \ + echo "Copying .so file to motion" && \ + cp $$(find ../.motionenv -name "motionstate*.so") ../motion/ && \ + echo "Cleanup virtual environment..." && \ + rm -rf ../.motionenv && \ + echo "Done" diff --git a/motion/__init__.py b/motion/__init__.py index cd3ed39a..0162211a 100644 --- a/motion/__init__.py +++ b/motion/__init__.py @@ -11,6 +11,7 @@ from motion.dicts import MDataFrame from motion.copy_utils import copy_db from motion.server.application import Application +from motionstate import StateValue, State __all__ = [ "Component", @@ -24,4 +25,6 @@ "copy_db", "RedisParams", "Application", + "StateValue", + "State", ] diff --git a/rustystate/Cargo.toml b/motionstate/Cargo.toml similarity index 55% rename from rustystate/Cargo.toml rename to motionstate/Cargo.toml index ca0ed096..d85b2ab7 100644 --- a/rustystate/Cargo.toml +++ b/motionstate/Cargo.toml @@ -1,10 +1,13 @@ [package] -name = "rustystate" +name = "motionstate" version = "0.1.0" edition = "2021" # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html +[lib] +crate-type = ["cdylib"] + [dependencies] -redis = "0.19" -pyo3 = { version = "0.15", features = ["extension-module"] } +redis = "0.23" +pyo3 = { version = "0.19", features = ["extension-module"] } diff --git a/rustystate/src/lib.rs b/motionstate/src/lib.rs similarity index 64% rename from rustystate/src/lib.rs rename to motionstate/src/lib.rs index 0c668d3f..84b9c0ad 100644 --- a/rustystate/src/lib.rs +++ b/motionstate/src/lib.rs @@ -1,3 +1,6 @@ +pub mod state_value; +use state_value::StateValue; + use pyo3::exceptions; use pyo3::prelude::*; use pyo3::types::{PyAny, PyBytes, PyDict, PyFloat, PyInt, PyList, PyString}; @@ -7,7 +10,8 @@ use std::collections::HashMap; /* TODO: * Increment version when calling set_bulk -* Remove set method (unnecessary) +* Use proper keys with component and instance names +* Construct redis_url out of redis params */ #[pyclass] @@ -36,13 +40,17 @@ impl State { }) } - pub fn set(&mut self, py: Python, key: String, value: &PyAny) -> PyResult<()> { + pub fn set(&mut self, py: Python, key: &str, value: &PyAny) -> PyResult<()> { let mut con = self.client.get_connection().unwrap(); - let serialized_data = serialize_value(py, value)?; - self.cache.insert(key.clone(), serialized_data.clone()); - con.set::<_, _, ()>(key, serialized_data).unwrap(); + // Insert the key and value into the cache + self.cache.insert(key.to_string(), serialized_data.clone()); + // Insert the key and value into Redis + con.set(key, serialized_data).map_err(|err| { + PyErr::new::(format!("Redis set error: {}", err)) + })?; + Ok(()) } @@ -97,11 +105,16 @@ impl State { ))), } } + + pub fn clear_cache(&mut self) { + self.cache.clear(); + } } // Serialization Helpers const MARKER_LIST: u8 = 0x01; const MARKER_DICT: u8 = 0x02; +const MARKER_STATE_VALUE: u8 = 0x03; fn cloudpickle_serialize(py: Python, value: &PyAny) -> PyResult> { let cloudpickle = py.import("cloudpickle")?; @@ -120,16 +133,16 @@ fn cloudpickle_deserialize(py: Python, value: &[u8]) -> PyResult { } fn serialize_value(py: Python, value: &PyAny) -> PyResult> { - if value.is_instance::()? - || value.is_instance::()? - || value.is_instance::()? + if value.is_instance_of::() + || value.is_instance_of::() + || value.is_instance_of::() { Ok(value.str()?.to_string().into_bytes()) - } else if value.is_instance::()? { + } else if value.is_instance_of::() { let mut serialized = vec![MARKER_DICT]; serialized.extend(serialize_dict(py, value)?); Ok(serialized) - } else if value.is_instance::()? { + } else if value.is_instance_of::() { let list = value.downcast::()?; let mut serialized = vec![MARKER_LIST]; for item in list.iter() { @@ -138,6 +151,20 @@ fn serialize_value(py: Python, value: &PyAny) -> PyResult> { serialized.extend(serialized_item); } Ok(serialized) + } else if value.is_instance_of::() { + let mut serialized = vec![MARKER_STATE_VALUE]; + + let saved_data = value.call_method0("save")?; + + // Check if the saved_data is bytes + let bytes_data = saved_data.downcast::().map_err(|_| { + PyErr::new::( + "'save' method must return a bytes object", + ) + })?; + + serialized.extend(bytes_data.as_bytes()); + Ok(serialized) } else { cloudpickle_serialize(py, value) } @@ -181,6 +208,13 @@ fn deserialize_value(py: Python, value: &[u8]) -> PyResult { let mut cursor = 0; match value[cursor] { + MARKER_STATE_VALUE => { + cursor += 1; + let state_value_data = &value[cursor..]; + let state_value_type = py.get_type::(); + let result = state_value_type.call_method1("load", (state_value_data,))?; + Ok(result.into()) + } MARKER_LIST => { cursor += 1; let list = pyo3::types::PyList::empty(py); @@ -218,99 +252,62 @@ fn deserialize_value(py: Python, value: &[u8]) -> PyResult { } } -// fn serialize_list(py: Python, value: &PyAny) -> PyResult>> { -// let list = value.downcast::()?; -// let mut serialized_items = Vec::new(); - -// for item in list.iter() { -// if item.is_instance::()? { -// serialized_items.push(item.str()?.to_string()); -// } else if item.is_instance::()? { -// serialized_items.push(item.str()?.to_string()); -// } else if item.is_instance::()? { -// serialized_items.push(item.str()?.to_string()); -// } else { -// // If the list contains non-primitive types, return None -// return Ok(None); -// } -// } - -// // Joining the serialized items with a delimiter (e.g., `|`), you can choose another delimiter if you wish. -// Ok(Some(serialized_items.join("|").into_bytes())) -// } - -// fn serialize_value(py: Python, value: &PyAny) -> PyResult> { -// if value.is_instance::()? -// || value.is_instance::()? -// || value.is_instance::()? -// { -// Ok(value.str()?.to_string().into_bytes()) -// } else if value.is_instance::()? { -// if let Some(serialized) = serialize_list(py, value)? { -// Ok(serialized) -// } else { -// // If couldn't serialize as a list of primitives, use cloudpickle -// let cloudpickle = py.import("cloudpickle")?; -// let bytes = cloudpickle -// .getattr("dumps")? -// .call1((value,))? -// .extract::<&PyBytes>()?; -// Ok(bytes.as_bytes().to_vec()) -// } -// } else { -// // Use cloudpickle for other types -// let cloudpickle = py.import("cloudpickle")?; -// let bytes = cloudpickle -// .getattr("dumps")? -// .call1((value,))? -// .extract::<&PyBytes>()?; -// Ok(bytes.as_bytes().to_vec()) -// } -// } - -// fn deserialize_value(py: Python, value: &[u8]) -> PyResult { -// if let Ok(decoded) = std::str::from_utf8(value) { -// if let Ok(int_value) = decoded.parse::() { -// Ok(int_value.into_py(py)) -// } else if let Ok(float_value) = decoded.parse::() { -// Ok(float_value.into_py(py)) -// } else if decoded.contains("|") { -// // Detecting our delimiter to identify lists -// let items: Vec<_> = decoded -// .split('|') -// .map(|item| { -// if let Ok(int_value) = item.parse::() { -// int_value.into_py(py) -// } else if let Ok(float_value) = item.parse::() { -// float_value.into_py(py) -// } else { -// item.to_string().into_py(py) -// } -// }) -// .collect(); -// Ok(PyList::new(py, &items).into()) -// } else { -// Ok(decoded.to_string().into_py(py)) -// } -// } else { -// let cloudpickle = py.import("cloudpickle")?; -// let bytes_value = PyBytes::new(py, value); -// let obj = cloudpickle.getattr("loads")?.call1((bytes_value,))?; -// Ok(obj.into()) -// } -// } - #[pymodule] -fn rustystate(_py: Python, m: &PyModule) -> PyResult<()> { +fn motionstate(_py: Python, m: &PyModule) -> PyResult<()> { m.add_class::()?; + m.add_class::()?; Ok(()) } #[cfg(test)] mod tests { use super::*; + use pyo3::Python; + + #[test] + fn state_init_with_valid_url() { + let _state = State::new( + "component".to_string(), + "instance".to_string(), + "redis://127.0.0.1:6381", + ) + .unwrap(); + } + + #[test] + fn state_init_with_invalid_url() { + let result = State::new( + "component".to_string(), + "instance".to_string(), + "invalid_url", + ); + assert!(result.is_err()); + } + #[test] - fn it_works() { - assert_eq!(2 + 2, 4); + fn cache_test() { + let gil = Python::acquire_gil(); + let py = gil.python(); + + let mut state = State::new( + "component".to_string(), + "instance".to_string(), + "redis://127.0.0.1:6381", + ) + .unwrap(); + + // Set a value to Redis + let _ = state + .bulk_set(py, [("test_key", 42)].into_py_dict(py)) + .unwrap(); + + // Clear cache to simulate fetching from Redis + state.clear_cache(); + let first_fetch = state.get(py, "test_key").unwrap(); + assert_eq!(first_fetch.extract::(py).unwrap(), 42); + + // This should be fetched from cache + let second_fetch = state.get(py, "test_key").unwrap(); + assert_eq!(second_fetch.extract::(py).unwrap(), 42); } } diff --git a/motionstate/src/state_value.rs b/motionstate/src/state_value.rs new file mode 100644 index 00000000..d918b393 --- /dev/null +++ b/motionstate/src/state_value.rs @@ -0,0 +1,41 @@ +use pyo3::exceptions::PyNotImplementedError; +use pyo3::prelude::*; +use pyo3::types::{PyBytes, PyType}; + +#[pyclass] +pub struct StateValue; + +#[pymethods] +impl StateValue { + #[classmethod] + pub fn load(_cls: &PyType, _data: &PyBytes) -> PyResult<()> { + Err(PyNotImplementedError::new_err( + "The 'load' method has not been implemented.", + )) + } + + pub fn save(&self, _py: Python) -> PyResult<&PyBytes> { + Err(PyNotImplementedError::new_err( + "The 'save' method has not been implemented.", + )) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_not_implemented() { + let gil = Python::acquire_gil(); + let py = gil.python(); + + let state_object = py.get_type::(); + let result = state_object.call_method1("load", ("some_data",)); + assert!(result.is_err()); + + let obj = state_object.call0().unwrap(); + let result = obj.call_method0("save"); + assert!(result.is_err()); + } +} diff --git a/rustystate/pyproject.toml b/rustystate/pyproject.toml deleted file mode 100644 index 7cc6ceaa..00000000 --- a/rustystate/pyproject.toml +++ /dev/null @@ -1,9 +0,0 @@ -[build-system] -requires = ["maturin"] -build-backend = "maturin" - -[project] -name = "rustystate" -version = "0.1.0" -authors = [{name = "Shreya Shankar", email = "ss.shankar505@gmail.com"}] -description = "A rust-state Python extension" From de7bbce390c1305c130ed4d9c572688c1b1d68f9 Mon Sep 17 00:00:00 2001 From: Shreya Shankar Date: Fri, 25 Aug 2023 17:11:31 -0700 Subject: [PATCH 05/37] Rebase with main --- Makefile | 4 +- motion/component.py | 92 ++++++------ motion/dicts.py | 89 +++++++++-- motion/execute.py | 242 ++++++++++++------------------ motion/instance.py | 15 +- motion/migrate.py | 53 ++++--- motion/route.py | 2 +- motion/server/update_task.py | 70 +++++---- motion/state/__init__.py | 4 + motion/state/state.py | 150 +++++++++++++++++++ motion/utils.py | 147 +++++++++--------- motionstate/src/lib.rs | 266 ++++++++++++++++++++++++++------- motionstate/src/state_value.rs | 17 +-- tests/state/test_db_conn.py | 31 ++-- tests/test_migrate.py | 18 ++- 15 files changed, 790 insertions(+), 410 deletions(-) create mode 100644 motion/state/__init__.py create mode 100644 motion/state/state.py diff --git a/Makefile b/Makefile index 1a58c843..7bf1d008 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,4 @@ -.PHONY: tests lint install mypy update docs statebuild +.PHONY: tests lint install mypy update docs build tests: poetry run pytest @@ -19,7 +19,7 @@ update: docs: poetry run mkdocs serve -statebuild: +build: cd motionstate && \ echo "Building motionstate" && \ maturin build --release && \ diff --git a/motion/component.py b/motion/component.py index fc032196..77559743 100644 --- a/motion/component.py +++ b/motion/component.py @@ -147,8 +147,8 @@ def __init__( self._serve_routes: Dict[str, Route] = {} self._update_routes: Dict[str, List[Route]] = {} self._init_state_func: Optional[Callable] = None - self._save_state_func: Optional[Callable] = None - self._load_state_func: Optional[Callable] = None + # self._save_state_func: Optional[Callable] = None + # self._load_state_func: Optional[Callable] = None @property def cache_ttl(self) -> int: @@ -245,59 +245,59 @@ def setUp(): self._init_state_func = func return func - def save_state(self, func: Callable) -> Callable: - """Decorator for the save_state function. This function - saves the state of the component to be accessible in - future component instances of the same name. + # def save_state(self, func: Callable) -> Callable: + # """Decorator for the save_state function. This function + # saves the state of the component to be accessible in + # future component instances of the same name. - Usage: - ```python - from motion import Component + # Usage: + # ```python + # from motion import Component - MyComponent = Component("MyComponent") + # MyComponent = Component("MyComponent") - @c.save_state - def save(state): - # state might have other unpicklable keys, like a DB connection - return {"fit_count": state["fit_count"]} - ``` + # @c.save_state + # def save(state): + # # state might have other unpicklable keys, like a DB connection + # return {"fit_count": state["fit_count"]} + # ``` - Args: - func (Callable): Function that returns a cloudpickleable object. + # Args: + # func (Callable): Function that returns a cloudpickleable object. - Returns: - Callable: Decorated save_state function. - """ - self._save_state_func = func - return func + # Returns: + # Callable: Decorated save_state function. + # """ + # self._save_state_func = func + # return func - def load_state(self, func: Callable) -> Callable: - """Decorator for the load_state function. This function - loads the state of the component from the unpickled state. + # def load_state(self, func: Callable) -> Callable: + # """Decorator for the load_state function. This function + # loads the state of the component from the unpickled state. - Usage: - ```python - from motion import Component + # Usage: + # ```python + # from motion import Component - MyComponent = Component("MyComponent") + # MyComponent = Component("MyComponent") - @c.load_state - def load(state): - conn = sqlite3.connect(":memory:") - cursor = conn.cursor() - return {"cursor": cursor, "fit_count": state["fit_count"]} - ``` + # @c.load_state + # def load(state): + # conn = sqlite3.connect(":memory:") + # cursor = conn.cursor() + # return {"cursor": cursor, "fit_count": state["fit_count"]} + # ``` - Args: - func (Callable): Function that consumes a cloudpickleable object. - Should return a dictionary representing the state of the - component instance. + # Args: + # func (Callable): Function that consumes a cloudpickleable object. + # Should return a dictionary representing the state of the + # component instance. - Returns: - Callable: Decorated load_state function. - """ - self._load_state_func = func - return func + # Returns: + # Callable: Decorated load_state function. + # """ + # self._load_state_func = func + # return func def serve(self, keys: Union[str, List[str]]) -> Callable: """Decorator for any serve operation for a dataflow through the @@ -549,8 +549,8 @@ def ... instance_id=instance_id, init_state_func=self._init_state_func, init_state_params=init_state_params, - save_state_func=self._save_state_func, - load_state_func=self._load_state_func, + # save_state_func=self._save_state_func, + # load_state_func=self._load_state_func, serve_routes=self._serve_routes, update_routes=self._update_routes, logging_level=logging_level, diff --git a/motion/dicts.py b/motion/dicts.py index 6ad74f30..5762333f 100644 --- a/motion/dicts.py +++ b/motion/dicts.py @@ -2,7 +2,9 @@ This file contains the props class, which is used to store properties of a flow. """ -from typing import Any, Optional +from typing import Any, Iterator, List, Optional, Tuple + +from motionstate import StateAccessor import pandas as pd import pyarrow as pa @@ -87,14 +89,8 @@ def serve_result(self) -> Any: """ return self._serve_result - # def __getattr__(self, key: str) -> object: - # return self.__getitem__(key) - - # def __setattr__(self, key: str, value: Any) -> None: - # self[key] = value - # def __getstate__(self) -> dict: - # return dict(self) +STATE_ERROR_MSG = "Cannot edit state directly. Use component update operations instead." class State(dict): @@ -130,13 +126,28 @@ def __init__( self, component_name: str, instance_id: str, + redis_host: str, + redis_port: int, + redis_db: int = 0, + redis_password: Optional[str] = None, *args: Any, **kwargs: Any, ) -> None: self.component_name = component_name self._instance_id = instance_id + self._state_accessor = StateAccessor( + component_name, + instance_id, + redis_host, + redis_port, + redis_db, + redis_password, + ) super().__init__(*args, **kwargs) + def get_version(self) -> int: + return self._state_accessor.version + @property def instance_id(self) -> str: """ @@ -146,15 +157,75 @@ def instance_id(self) -> str: """ return self._instance_id + def clear_cache(self) -> None: + # Clear the cache + self._state_accessor.clear_cache() + def __getitem__(self, key: str) -> object: try: - return super().__getitem__(key) + # Get from state accessor + return self._state_accessor.get(key) except KeyError: raise KeyError( f"Key `{key}` not found in state for " + f"instance {self.component_name}__{self._instance_id}." ) + def __setitem__(self, key: str, value: Any) -> None: + # Disable this functionality + raise NotImplementedError(STATE_ERROR_MSG) + + def flushUpdateDict(self, update_dict: dict) -> None: + self._state_accessor.bulk_set(update_dict) + + def __delitem__(self, key: str) -> None: + raise NotImplementedError(STATE_ERROR_MSG) + + def update(self, *args: Any, **kwargs: Any) -> None: + raise NotImplementedError(STATE_ERROR_MSG) + + def clear(self) -> None: + raise NotImplementedError(STATE_ERROR_MSG) + + def pop(self, *args: Any, **kwargs: Any) -> None: + raise NotImplementedError(STATE_ERROR_MSG) + + def popitem(self, *args: Any, **kwargs: Any) -> None: + raise NotImplementedError(STATE_ERROR_MSG) + + def keys(self) -> List[str]: + return self._state_accessor.keys() + + def values(self) -> List[Any]: + """Values in the state dictionary. + + Note: This fetches all the values from the state. We + do not recommend using this method as it can be slow. + Consider accessing values directly via `state[key]`. + + Returns: + List[Any]: List of values in the state. + """ + + return self._state_accessor.values() + + def items(self) -> List[Tuple[str, Any]]: + """Items in the state dictionary. + + Note: This fetches all the key-value pairs from the state. + We do not recommend using this method as it can be slow. + If you need to iterate over the state, conditionally accessing + values, we recommend using the `keys` method instead and then + calling `state[key]` to access the value. + + Returns: + List[Tuple[str, Any]]: List of key-value pairs in the state. + """ + return self._state_accessor.items() + + def __iter__(self) -> Iterator[str]: + return self._state_accessor.__iter__() + class MDataFrame(pd.DataFrame): """Wrapper around pandas DataFrame that allows for pyarrow-based diff --git a/motion/execute.py b/motion/execute.py index acee3ecc..e83ba577 100644 --- a/motion/execute.py +++ b/motion/execute.py @@ -1,5 +1,4 @@ import asyncio -import logging import multiprocessing import threading from concurrent.futures import ThreadPoolExecutor @@ -7,20 +6,20 @@ from uuid import uuid4 import cloudpickle +import picologging as logging import psutil import redis -from motion.dicts import Properties, State +from motion.dicts import Properties from motion.route import Route from motion.server.update_task import UpdateProcess, UpdateThread +from motion.state import State from motion.utils import ( RedisParams, UpdateEvent, UpdateEventGroup, get_redis_params, hash_object, - loadState, - saveState, ) logger = logging.getLogger(__name__) @@ -33,8 +32,8 @@ def __init__( cache_ttl: int, init_state_func: Optional[Callable], init_state_params: Dict[str, Any], - save_state_func: Optional[Callable], - load_state_func: Optional[Callable], + # save_state_func: Optional[Callable], + # load_state_func: Optional[Callable], serve_routes: Dict[str, Route], update_routes: Dict[str, List[Route]], update_task_type: Literal["thread", "process"] = "thread", @@ -46,8 +45,6 @@ def __init__( self._init_state_func = init_state_func self._init_state_params = init_state_params - self._load_state_func = load_state_func - self._save_state_func = save_state_func self.running: Any = multiprocessing.Value("b", False) self._redis_socket_timeout = redis_socket_timeout @@ -65,28 +62,42 @@ def __init__( self.running.value = True # Set up state - self.version = self._redis_con.get(f"MOTION_VERSION:{self._instance_name}") self._state = State( - instance_name.split("__")[0], - instance_name.split("__")[1], - {}, + self._instance_name.split("__")[0], + self._instance_name.split("__")[1], + redis_host=self._redis_params.host, + redis_port=self._redis_params.port, + redis_db=self._redis_params.db, + redis_password=self._redis_params.password, ) - if self.version is None: - self.version = 1 - # Setup state - self._state.update(self.setUp(**self._init_state_params)) - saveState( - self._state, - self._redis_con, - self._instance_name, - self._save_state_func, - ) - else: - # Load state - self._state = self._loadState() - self.version = self._redis_con.get(f"MOTION_VERSION:{self._instance_name}") - - self.version = int(self.version) + # If it is version 0, then call the init_state_func + if self._state.get_version() == 0 and self._init_state_func is not None: + update_dict = self._init_state_func(**self._init_state_params) + self._state.flushUpdateDict(update_dict) + + # self.version = self._redis_con.get(f"MOTION_VERSION:{self._instance_name}") + # self._state = State( + # instance_name.split("__")[0], + # instance_name.split("__")[1], + # {}, + # ) + # if self.version is None: + # self.version = 1 + # # Setup state + # self._state.update(self.setUp(**self._init_state_params)) + # saveState( + # self._state, + # self._redis_con, + # self._instance_name, + # self._save_state_func, + # ) + # else: + # # Load state + # self._state = self._loadState() + # version_name = f"MOTION_VERSION:{self._instance_name}" + # self.version = self._redis_con.get(version_name) + + # self.version = int(self.version) # Set up routes self._serve_routes: Dict[str, Route] = serve_routes @@ -124,7 +135,8 @@ def _connectToRedis(self) -> Tuple[RedisParams, redis.Redis]: return rp, r def _loadState(self) -> State: - return loadState(self._redis_con, self._instance_name, self._load_state_func) + # Clear state cache + self._state.clear_cache() def setUp(self, **kwargs: Any) -> Dict[str, Any]: # Set up initial state @@ -161,8 +173,6 @@ def _build_fit_jobs(self) -> None: self.worker_task = update_cls( instance_name=self._instance_name, routes=self.route_dict_for_fit, - save_state_func=self._save_state_func, - load_state_func=self._load_state_func, queue_identifiers=self.queue_ids_for_fit, channel_identifiers=self.channel_dict_for_fit, redis_params=self._redis_params.dict(), @@ -197,8 +207,6 @@ def _monitor_process(self) -> None: self.worker_task = update_cls( instance_name=self._instance_name, routes=self.route_dict_for_fit, - save_state_func=self._save_state_func, - load_state_func=self._load_state_func, queue_identifiers=self.queue_ids_for_fit, channel_identifiers=self.channel_dict_for_fit, redis_params=self._redis_params.dict(), @@ -256,56 +264,32 @@ def shutdown(self, is_open: bool) -> None: self.monitor_thread.join() def _updateState( - self, - new_state: Dict[str, Any], - force_update: bool = True, - use_lock: bool = True, + self, new_state: Dict[str, Any], force_update: bool = True ) -> None: if not new_state: return - if not isinstance(new_state, dict): - raise TypeError("State should be a dict.") + # if not isinstance(new_state, dict): + # raise TypeError("State should be a dict.") # Get latest state - if use_lock: - with self._redis_con.lock( - f"MOTION_LOCK:{self._instance_name}", timeout=120 - ): - if force_update: - self._state = self._loadState() - self._state.update(new_state) - - # Save state to redis - saveState( - self._state, - self._redis_con, - self._instance_name, - self._save_state_func, - ) - - version = self._redis_con.get(f"MOTION_VERSION:{self._instance_name}") - if version is None: - raise ValueError("Version not found in Redis.") - self.version = int(version) - - else: - if force_update: - self._state = self._loadState() - self._state.update(new_state) - - # Save state to redis - saveState( - self._state, - self._redis_con, - self._instance_name, - self._save_state_func, - ) - - version = self._redis_con.get(f"MOTION_VERSION:{self._instance_name}") - if version is None: - raise ValueError("Version not found in Redis.") - self.version = int(version) + # with self._redis_con.lock( + # f"MOTION_LOCK:{self._instance_name}", timeout=120 + # ): + # self._state = self._loadState() + self._state.flushUpdateDict(new_state) + + # Save state to redis + # saveState( + # self._state, + # self._redis_con, + # self._instance_name, + # self._save_state_func, + # ) + + # self.version = self._redis_con.get( + # f"MOTION_VERSION:{self._instance_name}" + # ) def _enqueue_and_trigger_update( self, @@ -325,32 +309,21 @@ def _enqueue_and_trigger_update( if flush_update: route = self._update_routes[key][update_udf_name] - # Hold lock - - with self._redis_con.lock( - f"MOTION_LOCK:{self._instance_name}", timeout=120 - ): - try: - self._state = self._loadState() - - state_update = route.run( - state=self._state, - props=props, - ) - - if not isinstance(state_update, dict): - raise ValueError("State update must be a dict.") - else: - # Update state - self._updateState( - state_update, - force_update=False, - use_lock=False, - ) - except Exception as e: - raise RuntimeError( - "Error running update route in main process: " + str(e) - ) + try: + state_update = route.run( + state=self._state, + props=props, + ) + + if not isinstance(state_update, dict): + raise ValueError("State update must be a dict.") + else: + # Update state + self._updateState(state_update, force_update=False) + except Exception as e: + raise RuntimeError( + "Error running update route in main process: " + str(e) + ) else: # Enqueue update @@ -397,33 +370,24 @@ async def _async_enqueue_and_trigger_update( if flush_update: route = self._update_routes[key][update_udf_name] - with self._redis_con.lock( - f"MOTION_LOCK:{self._instance_name}", timeout=120 - ): - try: - self._state = self._loadState() - - state_update = route.run( - state=self._state, - props=props, - ) - - if asyncio.iscoroutine(state_update): - state_update = await state_update - - if not isinstance(state_update, dict): - raise ValueError("State update must be a dict.") - else: - # Update state - self._updateState( - state_update, - force_update=False, - use_lock=False, - ) - except Exception as e: - raise RuntimeError( - "Error running update route in main process: " + str(e) - ) + try: + state_update = route.run( + state=self._state, + props=props, + ) + + if asyncio.iscoroutine(state_update): + state_update = await state_update + + if not isinstance(state_update, dict): + raise ValueError("State update must be a dict.") + else: + # Update state + self._updateState(state_update, force_update=False) + except Exception as e: + raise RuntimeError( + "Error running update route in main process: " + str(e) + ) else: # Enqueue update @@ -463,14 +427,7 @@ def _try_cached_serve( serve_result = None if force_refresh: - self._state = self._loadState() - v = self._redis_con.get(f"MOTION_VERSION:{self._instance_name}") - if not v: - raise ValueError( - f"Error loading state for {self._instance_name}." - + " No version found." - ) - self.version = int(v) + self._loadState() # If caching is disabled, return if self._cache_ttl == 0: @@ -641,11 +598,4 @@ def flush_update(self, dataflow_key: str) -> None: # Wait for update result to finish update_events.wait() # Update state - self._state = self._loadState() - v = self._redis_con.get(f"MOTION_VERSION:{self._instance_name}") - if not v: - raise ValueError( - f"Error loading state for {self._instance_name}." + " No version found." - ) - - self.version = int(v) + self._loadState() diff --git a/motion/instance.py b/motion/instance.py index 0709dee9..67fe028e 100644 --- a/motion/instance.py +++ b/motion/instance.py @@ -1,7 +1,8 @@ import atexit -import logging from typing import Any, Awaitable, Callable, Dict, List, Literal, Optional +import picologging as logging + from motion.execute import Executor from motion.route import Route from motion.utils import DEFAULT_KEY_TTL, configureLogging @@ -27,8 +28,8 @@ def __init__( instance_id: str, init_state_func: Optional[Callable], init_state_params: Optional[Dict[str, Any]], - save_state_func: Optional[Callable], - load_state_func: Optional[Callable], + # save_state_func: Optional[Callable], + # load_state_func: Optional[Callable], serve_routes: Dict[str, Route], update_routes: Dict[str, List[Route]], logging_level: str = "WARNING", @@ -55,7 +56,7 @@ def __init__( configureLogging(logging_level) # self._serverless = serverless # indicator = "serverless" if serverless else "local" - logger.info(f"Creating local instance of {self._component_name}...") + logger.debug(f"Creating local instance of {self._component_name}...") atexit.register(self.shutdown) # Create instance name @@ -69,8 +70,8 @@ def __init__( cache_ttl=self._cache_ttl, init_state_func=init_state_func, init_state_params=init_state_params if init_state_params else {}, - save_state_func=save_state_func, - load_state_func=load_state_func, + # save_state_func=save_state_func, + # load_state_func=load_state_func, serve_routes=serve_routes, update_routes=update_routes, update_task_type=update_task_type, @@ -162,7 +163,7 @@ def setUp(): c_instance.get_version() # Returns 1 (first version) ``` """ - return self._executor.version # type: ignore + return self._executor._state.get_version() # type: ignore def write_state(self, state_update: Dict[str, Any], latest: bool = False) -> None: """Writes the state update to the component instance's state. diff --git a/motion/migrate.py b/motion/migrate.py index a4d7ebba..1f6e638c 100644 --- a/motion/migrate.py +++ b/motion/migrate.py @@ -1,15 +1,15 @@ import inspect -import logging from multiprocessing import Pool from typing import Callable, List, Optional, Tuple +import picologging as logging import redis from pydantic import BaseConfig, BaseModel, Field from tqdm import tqdm from motion.component import Component -from motion.dicts import State -from motion.utils import get_redis_params, loadState, saveState +from motion.state import State +from motion.utils import get_redis_params logger = logging.getLogger(__name__) @@ -17,31 +17,40 @@ def process_migration( instance_name: str, migrate_func: Callable, - load_state_fn: Callable, - save_state_fn: Callable, + timeout: int = 60, # 60-second timeout for lock ) -> Tuple[str, Optional[Exception]]: try: rp = get_redis_params() redis_con = redis.Redis( **rp.dict(), ) - state = loadState(redis_con, instance_name, load_state_fn) - new_state = migrate_func(state) - assert isinstance(new_state, dict), ( - "Migration function must return a dict." - + " Warning: partial progress may have been made!" - ) - empty_state = State( + + state = State( instance_name.split("__")[0], instance_name.split("__")[1], - {}, + redis_host=rp.host, + redis_port=rp.port, + redis_db=rp.db, + redis_password=rp.password, ) - empty_state.update(new_state) - saveState(empty_state, redis_con, instance_name, save_state_fn) + + # Acquire lock to prevent other writes during migration + with redis_con.lock(f"MOTION_LOCK:{instance_name}", timeout=timeout): + # state = loadState(redis_con, instance_name, load_state_fn) + state_updates = migrate_func(state) + + assert isinstance(state_updates, dict), ( + "Migration function must return a dict of updates." + + " Warning: partial progress may have been made!" + ) + state.flushUpdateDict(state_updates, from_migration=True) + # saveState(empty_state, redis_con, instance_name, save_state_fn) except Exception as e: if isinstance(e, AssertionError): raise e else: + # Log an error and continue + logger.error(f"Error migrating {instance_name}: {e}", exc_info=True) return instance_name, e redis_con.close() @@ -85,7 +94,10 @@ def __init__(self, component: Component, migrate_func: Callable) -> None: self.migrate_func = migrate_func def migrate( - self, instance_ids: List[str] = [], num_workers: int = 4 + self, + instance_ids: List[str] = [], + num_workers: int = 4, + lock_timeout: int = 60, ) -> List[MigrationResult]: """Performs the migrate_func for component instances' states. If instance_ids is empty, then migrate_func is performed for all @@ -98,6 +110,10 @@ def migrate( num_workers (int, optional): Number of workers to use for parallel processing the migration. Defaults to 4. + lock_timeout (int, optional): + Number of seconds to lock the state object during its migration + operation to prevent any other writes from other processes. + Defaults to 60. Returns: List[MigrationResult]: @@ -117,9 +133,10 @@ def migrate( ] if not instance_names: instance_names = [ - key.decode("utf-8").replace("MOTION_STATE:", "") # type: ignore - for key in redis_con.keys(f"MOTION_STATE:{self.component.name}__*") + key.decode("utf-8").replace("MOTION_VERSION:", "") + for key in redis_con.keys(f"MOTION_VERSION:{self.component.name}__*") ] + print(instance_names) if not instance_names: logger.warning(f"No instances for component {self.component.name} found.") diff --git a/motion/route.py b/motion/route.py index cbde151e..245e7ed7 100644 --- a/motion/route.py +++ b/motion/route.py @@ -1,7 +1,7 @@ import inspect -import logging from typing import Any, Callable, Dict +import picologging as logging from pydantic import BaseModel, Field, PrivateAttr logger = logging.getLogger(__name__) diff --git a/motion/server/update_task.py b/motion/server/update_task.py index 20733bbb..e1b97883 100644 --- a/motion/server/update_task.py +++ b/motion/server/update_task.py @@ -2,13 +2,14 @@ import traceback from multiprocessing import Process from threading import Thread -from typing import Any, Callable, Dict, List, Optional +from typing import Any, Dict, List, Optional import cloudpickle import redis from motion.route import Route -from motion.utils import loadState, logger, saveState +from motion.state import State +from motion.utils import logger class BaseUpdateTask: @@ -17,8 +18,6 @@ def __init__( task_type: str, instance_name: str, routes: Dict[str, Route], - save_state_func: Optional[Callable], - load_state_func: Optional[Callable], queue_identifiers: List[str], channel_identifiers: Dict[str, str], redis_params: Dict[str, Any], @@ -28,8 +27,6 @@ def __init__( self.task_type = task_type self.name = f"UpdateTask-{task_type}-{instance_name}" self.instance_name = instance_name - self.save_state_func = save_state_func - self.load_state_func = load_state_func self.routes = routes self.queue_identifiers = queue_identifiers @@ -94,32 +91,43 @@ def custom_run(self) -> None: # Run update op try: - with redis_con.lock(f"MOTION_LOCK:{self.instance_name}", timeout=120): - old_state = loadState( - redis_con, - self.instance_name, - self.load_state_func, - ) - state_update = self.routes[queue_name].run( - state=old_state, - props=item["props"], + # with redis_con.lock(f"MOTION_LOCK:{self.instance_name}", timeout=120): + # old_state = loadState( + # redis_con, + # self.instance_name, + # self.load_state_func, + # ) + + old_state = State( + self.instance_name.split("__")[0], + self.instance_name.split("__")[1], + redis_host=self.redis_host, + redis_port=self.redis_port, + redis_db=self.redis_db, + redis_password=self.redis_password, + ) + + state_update = self.routes[queue_name].run( + state=old_state, + props=item["props"], + ) + # Await if state_update is a coroutine + if asyncio.iscoroutine(state_update): + state_update = asyncio.run(state_update) + + if not isinstance(state_update, dict): + logger.error( + "Update methods should return a dict of state updates." ) - # Await if state_update is a coroutine - if asyncio.iscoroutine(state_update): - state_update = asyncio.run(state_update) - - if not isinstance(state_update, dict): - logger.error( - "Update methods should return a dict of state updates." - ) - else: - old_state.update(state_update) - saveState( - old_state, - redis_con, - self.instance_name, - self.save_state_func, - ) + else: + old_state.flushUpdateDict(state_update) + # old_state.update(state_update) + # saveState( + # old_state, + # redis_con, + # self.instance_name, + # self.save_state_func, + # ) except Exception: logger.error(traceback.format_exc()) exception_str = str(traceback.format_exc()) diff --git a/motion/state/__init__.py b/motion/state/__init__.py new file mode 100644 index 00000000..07a33b97 --- /dev/null +++ b/motion/state/__init__.py @@ -0,0 +1,4 @@ +from motion.state.state import State +from motionstate import TempValue + +__all__ = ["State", "TempValue"] diff --git a/motion/state/state.py b/motion/state/state.py new file mode 100644 index 00000000..52d6566f --- /dev/null +++ b/motion/state/state.py @@ -0,0 +1,150 @@ +""" +This file contains the props class, which is used to store +properties of a flow. +""" +from typing import Any, Iterator, List, Optional, Tuple + +from motionstate import StateAccessor + +STATE_ERROR_MSG = "Cannot edit state directly. Use component update operations instead." + + +class State: + """Python class that stores state for a component instance. + The instance id is stored in the `instance_id` attribute. + + Example usage: + + ```python + from motion import Component + + some_component = Component("SomeComponent") + + @some_component.init_state + def setUp(): + return {"model": ...} + + @some_component.serve("retrieve") + def retrieve_nn(state, props): + # model can be accessed via state["model"] + prediction = state["model"](props["image_embedding"]) + # match the prediction to some other data to do a retrieval + nn_component_instance = SomeOtherMotionComponent(state.instance_id) + return nn_component_instance.run("retrieve", props={"prediction": prediction}) + + if __name__ == "__main__": + c = some_component() + nearest_neighbors = c.run("retrieve", props={"image_embedding": ...}) + ``` + """ + + def __init__( + self, + component_name: str, + instance_id: str, + redis_host: str, + redis_port: int, + redis_db: int = 0, + redis_password: Optional[str] = None, + *args: Any, + **kwargs: Any, + ) -> None: + self.component_name = component_name + self._instance_id = instance_id + self._state_accessor = StateAccessor( + component_name, + instance_id, + 1000 * 60 * 2, # 2 minutes lock duration TODO: make this configurable + redis_host, + redis_port, + redis_db, + redis_password, + ) + super().__init__(*args, **kwargs) + + def get_version(self) -> int: + return self._state_accessor.version + + @property + def instance_id(self) -> str: + """ + Returns the instance id of the component. + Useful if wanting to create other component instances + within a serve or update operation. + """ + return self._instance_id + + def clear_cache(self) -> None: + # Clear the cache + self._state_accessor.clear_cache() + + def __getitem__(self, key: str) -> object: + try: + # Get from state accessor + return self._state_accessor.get(key) + except KeyError: + raise KeyError( + f"Key `{key}` not found in state for " + + f"instance {self.component_name}__{self._instance_id}." + ) + + def get(self, key: str, default: Optional[Any] = None) -> Any: + try: + return self[key] + except KeyError: + return default + + def __setitem__(self, key: str, value: Any) -> None: + # Disable this functionality + raise RuntimeError(STATE_ERROR_MSG) + + def flushUpdateDict(self, update_dict: dict, from_migration: bool = False) -> None: + self._state_accessor.bulk_set(update_dict, from_migration) + + def __delitem__(self, key: str) -> None: + raise RuntimeError(STATE_ERROR_MSG) + + def update(self, *args: Any, **kwargs: Any) -> None: + raise RuntimeError(STATE_ERROR_MSG) + + def clear(self) -> None: + raise RuntimeError(STATE_ERROR_MSG) + + def pop(self, *args: Any, **kwargs: Any) -> None: + raise RuntimeError(STATE_ERROR_MSG) + + def popitem(self, *args: Any, **kwargs: Any) -> None: + raise RuntimeError(STATE_ERROR_MSG) + + def keys(self) -> List[str]: + return self._state_accessor.keys() + + def values(self) -> List[Any]: + """Values in the state dictionary. + + Note: This fetches all the values from the state. We + do not recommend using this method as it can be slow. + Consider accessing values directly via `state[key]`. + + Returns: + List[Any]: List of values in the state. + """ + + return self._state_accessor.values() + + def items(self) -> List[Tuple[str, Any]]: + """Items in the state dictionary. + + Note: This fetches all the key-value pairs from the state. + We do not recommend using this method as it can be slow. + If you need to iterate over the state, conditionally accessing + values, we recommend using the `keys` method instead and then + calling `state[key]` to access the value. + + Returns: + List[Tuple[str, Any]]: List of key-value pairs in the state. + """ + return self._state_accessor.items() + + def __iter__(self) -> Iterator[str]: + return iter(self._state_accessor.keys()) diff --git a/motion/utils.py b/motion/utils.py index dca431ae..0a3252b1 100644 --- a/motion/utils.py +++ b/motion/utils.py @@ -1,17 +1,16 @@ import hashlib -import logging import os import random from pathlib import Path -from typing import Any, Callable, Dict, List, Optional +from typing import Any, Dict, List, Optional -import cloudpickle -import colorlog +import picologging as logging import redis import yaml from pydantic import BaseModel -from motion.dicts import CustomDict, State +from motion.dicts import CustomDict +from motion.state import State logger = logging.getLogger(__name__) @@ -214,10 +213,18 @@ def inspect_state(instance_name: str) -> Dict[str, Any]: raise ValueError(f"Instance {instance_name} does not exist.") # Get the state - state = loadState(redis_con, instance_name, None) + state = State( + instance_name.split("__")[0], + instance_name.split("__")[1], + rp.host, + rp.port, + rp.db, + rp.password, + ) + # Iterate through all items + all_items = {k: v for k, v in state.items()} - redis_con.close() - return state + return all_items def validate_args(parameters: Any, op: str) -> bool: @@ -234,68 +241,68 @@ def validate_args(parameters: Any, op: str) -> bool: def configureLogging(level: str) -> None: - formatter = colorlog.ColoredFormatter( - "%(log_color)s%(asctime)s %(levelname)-8s%(reset)s %(blue)s%(message)s", - datefmt="%Y-%m-%d %H:%M:%S", - log_colors={ - "DEBUG": "cyan", - "INFO": "green", - "WARNING": "yellow", - "ERROR": "red", - "CRITICAL": "bold_red", - }, - ) - - logger = logging.getLogger("motion") - if logger.hasHandlers(): - logger.handlers.clear() - - stream_handler = logging.StreamHandler() - stream_handler.setFormatter(formatter) - - logger.addHandler(stream_handler) - logger.setLevel(level) - - -def loadState( - redis_con: redis.Redis, - instance_name: str, - load_state_func: Optional[Callable], -) -> State: - # Get state from redis - state = State(instance_name.split("__")[0], instance_name.split("__")[1], {}) - loaded_state = redis_con.get(f"MOTION_STATE:{instance_name}") - - if not loaded_state: - # This is an error - logger.warning(f"Could not find state for {instance_name}.") - return state - - # Unpickle state - loaded_state = cloudpickle.loads(loaded_state) - - if load_state_func is not None: - state.update(load_state_func(loaded_state)) - else: - state.update(loaded_state) - - return state - - -def saveState( - state_to_save: State, - redis_con: redis.Redis, - instance_name: str, - save_state_func: Optional[Callable], -) -> None: - # Save state to redis - if save_state_func is not None: - state_to_save = save_state_func(state_to_save) - - state_pickled = cloudpickle.dumps(state_to_save) - - redis_con.set(f"MOTION_STATE:{instance_name}", state_pickled) - redis_con.incr(f"MOTION_VERSION:{instance_name}") + # formatter = colorlog.ColoredFormatter( + # "%(log_color)s%(asctime)s %(levelname)-8s%(reset)s %(blue)s%(message)s", + # datefmt="%Y-%m-%d %H:%M:%S", + # log_colors={ + # "DEBUG": "cyan", + # "INFO": "green", + # "WARNING": "yellow", + # "ERROR": "red", + # "CRITICAL": "bold_red", + # }, + # ) + logging.basicConfig(level=level) + # logger = logging.getLogger("motion") + # if logger.hasHandlers(): + # logger.handlers.clear() + + # stream_handler = logging.StreamHandler() + # stream_handler.setFormatter(formatter) + + # logger.addHandler(stream_handler) + # logger.setLevel(level) + + +# def loadState( +# redis_con: redis.Redis, +# instance_name: str, +# load_state_func: Optional[Callable], +# ) -> State: +# # Get state from redis +# state = State(instance_name.split("__")[0], instance_name.split("__")[1], {}) +# loaded_state = redis_con.get(f"MOTION_STATE:{instance_name}") + +# if not loaded_state: +# # This is an error +# logger.warning(f"Could not find state for {instance_name}.") +# return state + +# # Unpickle state +# loaded_state = cloudpickle.loads(loaded_state) + +# if load_state_func is not None: +# state.update(load_state_func(loaded_state)) +# else: +# state.update(loaded_state) + +# return state + + +# def saveState( +# state_to_save: State, +# redis_con: redis.Redis, +# instance_name: str, +# save_state_func: Optional[Callable], +# ) -> None: +# # Save state to redis +# if save_state_func is not None: +# state_to_save = save_state_func(state_to_save) + +# state_pickled = cloudpickle.dumps(state_to_save) + +# redis_con.set(f"MOTION_STATE:{instance_name}", state_pickled) +# redis_con.incr(f"MOTION_VERSION:{instance_name}") class UpdateEvent: diff --git a/motionstate/src/lib.rs b/motionstate/src/lib.rs index 84b9c0ad..071f0cfb 100644 --- a/motionstate/src/lib.rs +++ b/motionstate/src/lib.rs @@ -6,50 +6,108 @@ use pyo3::prelude::*; use pyo3::types::{PyAny, PyBytes, PyDict, PyFloat, PyInt, PyList, PyString}; use redis::Commands; use std::collections::HashMap; +use std::sync::Arc; /* TODO: -* Increment version when calling set_bulk -* Use proper keys with component and instance names -* Construct redis_url out of redis params +* Use a redis lock */ #[pyclass] -pub struct State { +pub struct StateAccessor { component_name: String, instance_id: String, + version: u64, client: redis::Client, - cache: HashMap>, + cache: HashMap>>, } #[pymethods] -impl State { +impl StateAccessor { #[new] - pub fn new(component_name: String, instance_id: String, redis_url: &str) -> PyResult { + pub fn new( + component_name: String, + instance_id: String, + redis_host: &str, + redis_port: u16, + redis_db: i64, + redis_password: Option<&str>, + ) -> PyResult { + // Constructing the Redis URL + let redis_url = match redis_password { + Some(password) => format!( + "redis://:{}@{}:{}/{}", + password, redis_host, redis_port, redis_db + ), + None => format!("redis://{}:{}/{}", redis_host, redis_port, redis_db), + }; + let client = redis::Client::open(redis_url).map_err(|err| { PyErr::new::(format!( "Redis connection error: {}", err )) })?; - Ok(State { + + // Read the version from Redis. If it doesn't exist, set it to 1. + let mut con = client.get_connection().unwrap(); + let instancename = format!("MOTION_VERSION:{}__{}", component_name, instance_id); + let version: u64 = con.get(&instancename).unwrap_or(1); + + Ok(StateAccessor { component_name, instance_id, + version, client, cache: HashMap::new(), }) } + #[getter] + pub fn version(&self) -> PyResult { + Ok(self.version) + } + pub fn set(&mut self, py: Python, key: &str, value: &PyAny) -> PyResult<()> { let mut con = self.client.get_connection().unwrap(); - let serialized_data = serialize_value(py, value)?; + let serialized_data = Arc::new(serialize_value(py, value)?); + + // Create key name as MOTION_STATE:__/ + let keyname = format!( + "MOTION_STATE:{}__{}/{}", + self.component_name, self.instance_id, key + ); // Insert the key and value into the cache - self.cache.insert(key.to_string(), serialized_data.clone()); - // Insert the key and value into Redis - con.set(key, serialized_data).map_err(|err| { - PyErr::new::(format!("Redis set error: {}", err)) - })?; + self.cache.insert(keyname.clone(), serialized_data.clone()); + + // Increment the version and write it to Redis + self.version += 1; + + // Insert the key and value into Redis through an atomic pipeline + redis::pipe() + .atomic() + .set(keyname.clone(), &*serialized_data) + .ignore() + .set( + format!( + "MOTION_VERSION:{}__{}", + self.component_name, self.instance_id + ), + self.version, + ) + .ignore() + .query(&mut con) + .map_err(|err| { + // Undo the cache insert and version increment + self.cache.remove(&keyname); + self.version -= 1; + + PyErr::new::(format!( + "Redis set data error: {}", + err + )) + })?; Ok(()) } @@ -57,21 +115,49 @@ impl State { pub fn bulk_set(&mut self, py: Python, items: &PyDict) -> PyResult<()> { let mut con = self.client.get_connection().unwrap(); let mut pipeline = redis::pipe(); + pipeline.atomic(); // Iterate over the items in the dictionary for (key, value) in items { - let key_str = key.extract::()?; - let serialized_data = serialize_value(py, value)?; + // Create key name as MOTION_STATE:__/ + let keyname = format!( + "MOTION_STATE:{}__{}/{}", + self.component_name, self.instance_id, key + ); + let serialized_data = Arc::new(serialize_value(py, value)?); // Insert the key and value into the cache - self.cache.insert(key_str.clone(), serialized_data.clone()); + self.cache.insert(keyname.clone(), serialized_data.clone()); // Insert the key and value into the pipeline - //pipeline.set::<_, _, ()>(key_str, serialized_data); - pipeline.cmd("SET").arg(key_str).arg(serialized_data); + //pipeline.set::<_, _, ()>(keyname, serialized_data); + + pipeline.cmd("SET").arg(keyname).arg(&*serialized_data); } + // Increment the version and write it to Redis + self.version += 1; + pipeline + .set( + format!( + "MOTION_VERSION:{}__{}", + self.component_name, self.instance_id + ), + self.version, + ) + .ignore(); + // Execute the pipeline, throwing a Python error if it fails - pipeline.query::<()>(&mut con).map_err(|err| { + pipeline.query(&mut con).map_err(|err| { + // Undo the cache insert and version increment + for (key, _) in items { + let keyname = format!( + "MOTION_STATE:{}__{}/{}", + self.component_name, self.instance_id, key + ); + self.cache.remove(&keyname); + } + self.version -= 1; + PyErr::new::(format!( "Redis bulk set error: {}", err @@ -82,21 +168,29 @@ impl State { } pub fn get(&mut self, py: Python, key: &str) -> PyResult { + // Create key name as MOTION_STATE:__/ + let keyname = format!( + "MOTION_STATE:{}__{}/{}", + self.component_name, self.instance_id, key + ); + // If the key is in the cache, return it - if let Some(value) = self.cache.get(key) { - return deserialize_value(py, value); + if let Some(value) = self.cache.get(&keyname) { + return deserialize_value(py, &*value); } // Otherwise, fetch it from Redis let mut con = self.client.get_connection().unwrap(); - let result_data: redis::RedisResult>> = con.get(key); + let result_data: redis::RedisResult>> = con.get(&keyname); match result_data { Ok(Some(data)) => { + let data_arc = Arc::new(data); + // Insert the key and value into the cache - self.cache.insert(key.to_string(), data.clone()); + self.cache.insert(keyname.clone(), data_arc.clone()); // Deserialize the value - deserialize_value(py, &data) + deserialize_value(py, &*data_arc) } Ok(None) => Err(PyErr::new::("Key not found")), Err(err) => Err(PyErr::new::(format!( @@ -106,6 +200,68 @@ impl State { } } + pub fn __iter__(&self, py: Python) -> PyResult { + let keys = self.keys(py)?; + let list = pyo3::types::PyList::new(py, &keys); + Ok(list.into()) + } + + pub fn items(&mut self, py: Python) -> PyResult { + let items_list = pyo3::types::PyList::empty(py); + let pattern = format!( + "MOTION_STATE:{}__{}/{}", + self.component_name, self.instance_id, "*" + ); + + let replaced_pattern = pattern.replace("*", ""); + let mut con = self.client.get_connection().unwrap(); + + // Minimized Redis calls by fetching everything in one go. + let keys: Vec = con.keys(pattern).map_err(|err| { + PyErr::new::(format!("Redis keys error: {}", err)) + })?; + + for key in keys { + let key_without_prefix = key.replace(&replaced_pattern, ""); + + // Avoid cloning the key for Python conversion. + let py_key = key_without_prefix.as_str().into_py(py); + let value = self.get(py, &key_without_prefix)?; + let tuple = pyo3::types::PyTuple::new(py, &[py_key, value]); + items_list.append(tuple)?; + } + + Ok(items_list.into()) + } + + pub fn keys(&self, _py: Python) -> PyResult> { + let pattern = format!( + "MOTION_STATE:{}__{}/{}", + self.component_name, self.instance_id, "*" + ); + + let mut con = self.client.get_connection().unwrap(); + let keys: Vec = con.keys(pattern.clone()).map_err(|err| { + PyErr::new::(format!("Redis keys error: {}", err)) + })?; + + let replaced_pattern = pattern.replace("*", ""); + Ok(keys + .into_iter() + .map(|key| key.replace(&replaced_pattern, "")) + .collect()) + } + + pub fn values(&mut self, py: Python) -> PyResult { + let values_list = pyo3::types::PyList::empty(py); + let keys = self.keys(py)?; + for key in keys.iter() { + let value = self.get(py, &key)?; + values_list.append(value)?; + } + Ok(values_list.into()) + } + pub fn clear_cache(&mut self) { self.cache.clear(); } @@ -254,7 +410,7 @@ fn deserialize_value(py: Python, value: &[u8]) -> PyResult { #[pymodule] fn motionstate(_py: Python, m: &PyModule) -> PyResult<()> { - m.add_class::()?; + m.add_class::()?; m.add_class::()?; Ok(()) } @@ -262,52 +418,60 @@ fn motionstate(_py: Python, m: &PyModule) -> PyResult<()> { #[cfg(test)] mod tests { use super::*; - use pyo3::Python; + use pyo3::types::IntoPyDict; #[test] fn state_init_with_valid_url() { - let _state = State::new( + let _state = StateAccessor::new( "component".to_string(), "instance".to_string(), - "redis://127.0.0.1:6381", + "127.0.0.1", + 6381, + 0, + None, ) .unwrap(); } #[test] fn state_init_with_invalid_url() { - let result = State::new( + let result = StateAccessor::new( "component".to_string(), "instance".to_string(), - "invalid_url", + "invalid", + 6381, + 0, + None, ); assert!(result.is_err()); } #[test] fn cache_test() { - let gil = Python::acquire_gil(); - let py = gil.python(); - - let mut state = State::new( - "component".to_string(), - "instance".to_string(), - "redis://127.0.0.1:6381", - ) - .unwrap(); - - // Set a value to Redis - let _ = state - .bulk_set(py, [("test_key", 42)].into_py_dict(py)) + pyo3::Python::with_gil(|py| { + let mut state = StateAccessor::new( + "component".to_string(), + "instance".to_string(), + "127.0.0.1", + 6381, + 0, + None, + ) .unwrap(); - // Clear cache to simulate fetching from Redis - state.clear_cache(); - let first_fetch = state.get(py, "test_key").unwrap(); - assert_eq!(first_fetch.extract::(py).unwrap(), 42); + // Set a value to Redis + let _ = state + .bulk_set(py, [("test_key", 42)].into_py_dict(py)) + .unwrap(); + + // Clear cache to simulate fetching from Redis + state.clear_cache(); + let first_fetch = state.get(py, "test_key").unwrap(); + assert_eq!(first_fetch.extract::(py).unwrap(), 42); - // This should be fetched from cache - let second_fetch = state.get(py, "test_key").unwrap(); - assert_eq!(second_fetch.extract::(py).unwrap(), 42); + // This should be fetched from cache + let second_fetch = state.get(py, "test_key").unwrap(); + assert_eq!(second_fetch.extract::(py).unwrap(), 42); + }); } } diff --git a/motionstate/src/state_value.rs b/motionstate/src/state_value.rs index d918b393..d20f1fbe 100644 --- a/motionstate/src/state_value.rs +++ b/motionstate/src/state_value.rs @@ -27,15 +27,14 @@ mod tests { #[test] fn test_not_implemented() { - let gil = Python::acquire_gil(); - let py = gil.python(); + pyo3::Python::with_gil(|py| { + let state_object = py.get_type::(); + let result = state_object.call_method1("load", ("some_data",)); + assert!(result.is_err()); - let state_object = py.get_type::(); - let result = state_object.call_method1("load", ("some_data",)); - assert!(result.is_err()); - - let obj = state_object.call0().unwrap(); - let result = obj.call_method0("save"); - assert!(result.is_err()); + let obj = state_object.call0().unwrap(); + let result = obj.call_method0("save"); + assert!(result.is_err()); + }); } } diff --git a/tests/state/test_db_conn.py b/tests/state/test_db_conn.py index a3fce2f2..cf471a53 100644 --- a/tests/state/test_db_conn.py +++ b/tests/state/test_db_conn.py @@ -1,6 +1,9 @@ +# This file tests the StateValue functionality + from motion import Component import sqlite3 +import os c = Component("DBComponent") @@ -8,7 +11,8 @@ @c.init_state def setUp(): # Create in-memory sqlite database - conn = sqlite3.connect(":memory:") + path = ":file::memory:?cache=shared:" + conn = sqlite3.connect(path) cursor = conn.cursor() cursor.execute( """CREATE TABLE IF NOT EXISTS users @@ -25,24 +29,14 @@ def setUp(): ) conn.commit() - return {"cursor": cursor, "fit_count": 0} - - -@c.save_state -def save(state): - return {"fit_count": state["fit_count"]} - - -@c.load_state -def load(state): - conn = sqlite3.connect(":memory:") - cursor = conn.cursor() - return {"cursor": cursor, "fit_count": state["fit_count"]} + return {"path": path, "fit_count": 0} @c.serve("count") def execute_fn(state, props): - return state["cursor"].execute("SELECT COUNT(*) FROM users").fetchall() + conn = sqlite3.connect(state["path"]) + cursor = conn.cursor() + return cursor.execute("SELECT COUNT(*) FROM users").fetchall() @c.serve("something") @@ -57,6 +51,11 @@ def increment(state, props): def test_db_component(): c_instance = c() - assert c_instance.run("count", props={"value": 1}) == [(2,)] + assert c_instance.run("count", props={"value": 1}, flush_update=True) == [ + (2,) + ] c_instance.run("something", props={"value": 1}, flush_update=True) assert c_instance.run("something", props={"value": 5}) == 1 + + # Delete the database + os.remove(c_instance.read_state("path")) diff --git a/tests/test_migrate.py b/tests/test_migrate.py index ac1ecaf9..06fbde0a 100644 --- a/tests/test_migrate.py +++ b/tests/test_migrate.py @@ -22,11 +22,15 @@ def migrator_not_returning_dict(state): return "this isn't a dict" -def good_migrator(state): +def bad_inplace_update(state): state.update({"another_val": 0}) return state +def good_migrator(state): + return {"another_val": 0} + + def test_state_migration(): # Create a bunch of instances instance_ids = [] @@ -42,9 +46,14 @@ def test_state_migration(): sm = StateMigrator(Something, migrator_not_returning_dict) sm.migrate() - # Run good migrator + # Run another bad migrator + sm = StateMigrator(Something, bad_inplace_update) + result = sm.migrate([instance_ids[0]], num_workers=1) + assert result[0].exception is not None + + # Run good migrator on one instance sm = StateMigrator(Something, good_migrator) - result = sm.migrate([instance_ids[0]]) + result = sm.migrate([instance_ids[0]], num_workers=1) assert len(result) == 1 assert result[0].instance_id == instance_ids[0] assert result[0].exception is None @@ -59,4 +68,5 @@ def test_state_migration(): # Assert the instances have the new state for instance_id in instance_ids: s = Something(instance_id) - assert s._executor._state == {"state_val": 0, "another_val": 0} + assert s._executor._state["state_val"] == 0 + assert s._executor._state["another_val"] == 0 From 10eacbf7818dfc7b6c8d29a5de334f92a90a5edb Mon Sep 17 00:00:00 2001 From: Shreya Shankar Date: Fri, 25 Aug 2023 17:14:59 -0700 Subject: [PATCH 06/37] Rebase with main --- Makefile | 1 + docs/api/component.md | 2 - motion/dicts.py | 23 ++++--- motionstate/Cargo.toml | 1 + motionstate/src/lib.rs | 135 ++++++++++++++++++++++++++++++++++++----- 5 files changed, 137 insertions(+), 25 deletions(-) diff --git a/Makefile b/Makefile index 7bf1d008..a16ea313 100644 --- a/Makefile +++ b/Makefile @@ -22,6 +22,7 @@ docs: build: cd motionstate && \ echo "Building motionstate" && \ + maturin develop && \ maturin build --release && \ python -m venv ../.motionenv && \ source ../.motionenv/bin/activate && pip install target/wheels/motionstate*.whl && deactivate && \ diff --git a/docs/api/component.md b/docs/api/component.md index 064d7ecf..1dcbc46a 100644 --- a/docs/api/component.md +++ b/docs/api/component.md @@ -7,8 +7,6 @@ - update - init_state - __call__ - - save_state - - load_state - name - params show_root_full_path: false diff --git a/motion/dicts.py b/motion/dicts.py index 5762333f..863a1b07 100644 --- a/motion/dicts.py +++ b/motion/dicts.py @@ -93,8 +93,8 @@ def serve_result(self) -> Any: STATE_ERROR_MSG = "Cannot edit state directly. Use component update operations instead." -class State(dict): - """Dictionary that stores state for a component instance. +class State: + """Python class that stores state for a component instance. The instance id is stored in the `instance_id` attribute. Example usage: @@ -138,6 +138,7 @@ def __init__( self._state_accessor = StateAccessor( component_name, instance_id, + 1000 * 60 * 2, # 2 minutes lock duration TODO: make this configurable redis_host, redis_port, redis_db, @@ -171,27 +172,33 @@ def __getitem__(self, key: str) -> object: + f"instance {self.component_name}__{self._instance_id}." ) + def get(self, key: str, default: Optional[Any] = None) -> Any: + try: + return self[key] + except KeyError: + return default + def __setitem__(self, key: str, value: Any) -> None: # Disable this functionality - raise NotImplementedError(STATE_ERROR_MSG) + raise RuntimeError(STATE_ERROR_MSG) def flushUpdateDict(self, update_dict: dict) -> None: self._state_accessor.bulk_set(update_dict) def __delitem__(self, key: str) -> None: - raise NotImplementedError(STATE_ERROR_MSG) + raise RuntimeError(STATE_ERROR_MSG) def update(self, *args: Any, **kwargs: Any) -> None: - raise NotImplementedError(STATE_ERROR_MSG) + raise RuntimeError(STATE_ERROR_MSG) def clear(self) -> None: - raise NotImplementedError(STATE_ERROR_MSG) + raise RuntimeError(STATE_ERROR_MSG) def pop(self, *args: Any, **kwargs: Any) -> None: - raise NotImplementedError(STATE_ERROR_MSG) + raise RuntimeError(STATE_ERROR_MSG) def popitem(self, *args: Any, **kwargs: Any) -> None: - raise NotImplementedError(STATE_ERROR_MSG) + raise RuntimeError(STATE_ERROR_MSG) def keys(self) -> List[str]: return self._state_accessor.keys() diff --git a/motionstate/Cargo.toml b/motionstate/Cargo.toml index d85b2ab7..b07b0571 100644 --- a/motionstate/Cargo.toml +++ b/motionstate/Cargo.toml @@ -11,3 +11,4 @@ crate-type = ["cdylib"] [dependencies] redis = "0.23" pyo3 = { version = "0.19", features = ["extension-module"] } +redlock = "2.0.0" diff --git a/motionstate/src/lib.rs b/motionstate/src/lib.rs index 071f0cfb..441130ff 100644 --- a/motionstate/src/lib.rs +++ b/motionstate/src/lib.rs @@ -5,6 +5,7 @@ use pyo3::exceptions; use pyo3::prelude::*; use pyo3::types::{PyAny, PyBytes, PyDict, PyFloat, PyInt, PyList, PyString}; use redis::Commands; +use redlock::RedLock; use std::collections::HashMap; use std::sync::Arc; @@ -17,9 +18,12 @@ TODO: pub struct StateAccessor { component_name: String, instance_id: String, + lock_duration: usize, version: u64, client: redis::Client, cache: HashMap>>, + lock_manager: RedLock, + max_lock_attempts: u32, } #[pymethods] @@ -28,6 +32,7 @@ impl StateAccessor { pub fn new( component_name: String, instance_id: String, + lock_duration: u64, redis_host: &str, redis_port: u16, redis_db: i64, @@ -42,24 +47,31 @@ impl StateAccessor { None => format!("redis://{}:{}/{}", redis_host, redis_port, redis_db), }; - let client = redis::Client::open(redis_url).map_err(|err| { + let client = redis::Client::open(redis_url.clone()).map_err(|err| { PyErr::new::(format!( "Redis connection error: {}", err )) })?; - // Read the version from Redis. If it doesn't exist, set it to 1. + // Read the version from Redis. If it doesn't exist, set it to 0. let mut con = client.get_connection().unwrap(); let instancename = format!("MOTION_VERSION:{}__{}", component_name, instance_id); - let version: u64 = con.get(&instancename).unwrap_or(1); + let version: u64 = con.get(&instancename).unwrap_or(0); + + // Create a lock manager + let lock_manager = RedLock::new(vec![redis_url]); + let max_lock_attempts = 3; Ok(StateAccessor { component_name, instance_id, + lock_duration: lock_duration.try_into().unwrap(), version, client, cache: HashMap::new(), + lock_manager, + max_lock_attempts, }) } @@ -78,6 +90,42 @@ impl StateAccessor { self.component_name, self.instance_id, key ); + // Acquire the lock using rslock + // Lockname will be MOTION_LOCK:__ + let lock_name = format!("MOTION_LOCK:{}__{}", self.component_name, self.instance_id); + let mut lock = None; + + // Loop until we get the lock + for _ in 0..self.max_lock_attempts { + match self + .lock_manager + .lock(lock_name.as_bytes(), self.lock_duration) + { + Ok(Some(l)) => { + lock = Some(l); + break; + } + Ok(None) => { + // Lock was not acquired. Sleep for 100ms and try again. + std::thread::sleep(std::time::Duration::from_millis(100)); + } + Err(e) => { + // Handle the Redis error, maybe return an error or log it. + return Err(PyErr::new::(format!( + "Failed to acquire lock due to Redis error: {}", + e + ))); + } + } + } + if lock.is_none() { + return Err(PyErr::new::(format!( + "Failed to acquire lock after {} attempts", + self.max_lock_attempts + ))); + } + + // Critical section // Insert the key and value into the cache self.cache.insert(keyname.clone(), serialized_data.clone()); @@ -103,35 +151,78 @@ impl StateAccessor { self.cache.remove(&keyname); self.version -= 1; + // Drop the lock + self.lock_manager.unlock(lock.as_ref().unwrap()); + PyErr::new::(format!( "Redis set data error: {}", err )) })?; + // Drop the lock + self.lock_manager.unlock(lock.as_ref().unwrap()); + Ok(()) } pub fn bulk_set(&mut self, py: Python, items: &PyDict) -> PyResult<()> { let mut con = self.client.get_connection().unwrap(); - let mut pipeline = redis::pipe(); - pipeline.atomic(); - // Iterate over the items in the dictionary - for (key, value) in items { - // Create key name as MOTION_STATE:__/ + // Preserialize all the data + let mut serialized_items = Vec::with_capacity(items.len()); + for (key, value) in items.iter() { let keyname = format!( "MOTION_STATE:{}__{}/{}", self.component_name, self.instance_id, key ); let serialized_data = Arc::new(serialize_value(py, value)?); + serialized_items.push((keyname, serialized_data)); + } + let mut pipeline = redis::pipe(); + pipeline.atomic(); + + // Acquire the lock using rslock + // Lockname will be MOTION_LOCK:__ + let lock_name = format!("MOTION_LOCK:{}__{}", self.component_name, self.instance_id); + let mut lock = None; + + // Loop until we get the lock + for _ in 0..self.max_lock_attempts { + match self + .lock_manager + .lock(lock_name.as_bytes(), self.lock_duration) + { + Ok(Some(l)) => { + lock = Some(l); + break; + } + Ok(None) => { + // Lock was not acquired. Sleep for 100ms and try again. + std::thread::sleep(std::time::Duration::from_millis(100)); + } + Err(e) => { + // Handle the Redis error, maybe return an error or log it. + return Err(PyErr::new::(format!( + "Failed to acquire lock due to Redis error: {}", + e + ))); + } + } + } + if lock.is_none() { + return Err(PyErr::new::(format!( + "Failed to acquire lock after {} attempts", + self.max_lock_attempts + ))); + } + + // Critical section + for (keyname, serialized_data) in serialized_items.iter() { // Insert the key and value into the cache self.cache.insert(keyname.clone(), serialized_data.clone()); - // Insert the key and value into the pipeline - //pipeline.set::<_, _, ()>(keyname, serialized_data); - - pipeline.cmd("SET").arg(keyname).arg(&*serialized_data); + pipeline.cmd("SET").arg(keyname).arg(&**serialized_data); } // Increment the version and write it to Redis @@ -158,12 +249,18 @@ impl StateAccessor { } self.version -= 1; + // Drop the lock + self.lock_manager.unlock(lock.as_ref().unwrap()); + PyErr::new::(format!( "Redis bulk set error: {}", err )) })?; + // Drop the lock + self.lock_manager.unlock(lock.as_ref().unwrap()); + Ok(()) } @@ -264,6 +361,15 @@ impl StateAccessor { pub fn clear_cache(&mut self) { self.cache.clear(); + + // Reset version to whatever is in Redis + let mut con = self.client.get_connection().unwrap(); + let version_key = format!( + "MOTION_VERSION:{}__{}", + self.component_name, self.instance_id + ); + let version: u64 = con.get(version_key).unwrap_or(0); + self.version = version; } } @@ -357,9 +463,8 @@ fn extract_next_slice<'a>(value: &'a [u8], cursor: &mut usize) -> Option<&'a [u8 fn deserialize_value(py: Python, value: &[u8]) -> PyResult { if value.is_empty() { - return Err(PyErr::new::( - "Empty data", - )); + // Return empty string for empty data + return Ok("".to_object(py)); } let mut cursor = 0; From 56bd45dffb21f54741792463d15cf176348e11a5 Mon Sep 17 00:00:00 2001 From: Shreya Shankar Date: Fri, 25 Aug 2023 17:15:53 -0700 Subject: [PATCH 07/37] Rebase with main --- motion/dicts.py | 294 +++++++++++++++++---------------- motionstate/src/lib.rs | 225 ++++++++++++++++--------- motionstate/src/state_value.rs | 9 +- motionstate/src/temp_value.rs | 54 ++++++ tests/state/test_temp_value.py | 29 ++++ 5 files changed, 391 insertions(+), 220 deletions(-) create mode 100644 motionstate/src/temp_value.rs create mode 100644 tests/state/test_temp_value.py diff --git a/motion/dicts.py b/motion/dicts.py index 863a1b07..eab12cf7 100644 --- a/motion/dicts.py +++ b/motion/dicts.py @@ -2,9 +2,9 @@ This file contains the props class, which is used to store properties of a flow. """ -from typing import Any, Iterator, List, Optional, Tuple +from typing import Any, Optional -from motionstate import StateAccessor +# from motionstate import StateAccessor import pandas as pd import pyarrow as pa @@ -90,148 +90,154 @@ def serve_result(self) -> Any: return self._serve_result -STATE_ERROR_MSG = "Cannot edit state directly. Use component update operations instead." - - -class State: - """Python class that stores state for a component instance. - The instance id is stored in the `instance_id` attribute. - - Example usage: - - ```python - from motion import Component - - some_component = Component("SomeComponent") - - @some_component.init_state - def setUp(): - return {"model": ...} - - @some_component.serve("retrieve") - def retrieve_nn(state, props): - # model can be accessed via state["model"] - prediction = state["model"](props["image_embedding"]) - # match the prediction to some other data to do a retrieval - nn_component_instance = SomeOtherMotionComponent(state.instance_id) - return nn_component_instance.run("retrieve", props={"prediction": prediction}) - - if __name__ == "__main__": - c = some_component() - nearest_neighbors = c.run("retrieve", props={"image_embedding": ...}) - ``` - """ - - def __init__( - self, - component_name: str, - instance_id: str, - redis_host: str, - redis_port: int, - redis_db: int = 0, - redis_password: Optional[str] = None, - *args: Any, - **kwargs: Any, - ) -> None: - self.component_name = component_name - self._instance_id = instance_id - self._state_accessor = StateAccessor( - component_name, - instance_id, - 1000 * 60 * 2, # 2 minutes lock duration TODO: make this configurable - redis_host, - redis_port, - redis_db, - redis_password, - ) - super().__init__(*args, **kwargs) - - def get_version(self) -> int: - return self._state_accessor.version - - @property - def instance_id(self) -> str: - """ - Returns the instance id of the component. - Useful if wanting to create other component instances - within a serve or update operation. - """ - return self._instance_id - - def clear_cache(self) -> None: - # Clear the cache - self._state_accessor.clear_cache() - - def __getitem__(self, key: str) -> object: - try: - # Get from state accessor - return self._state_accessor.get(key) - except KeyError: - raise KeyError( - f"Key `{key}` not found in state for " - + f"instance {self.component_name}__{self._instance_id}." - ) - - def get(self, key: str, default: Optional[Any] = None) -> Any: - try: - return self[key] - except KeyError: - return default - - def __setitem__(self, key: str, value: Any) -> None: - # Disable this functionality - raise RuntimeError(STATE_ERROR_MSG) - - def flushUpdateDict(self, update_dict: dict) -> None: - self._state_accessor.bulk_set(update_dict) - - def __delitem__(self, key: str) -> None: - raise RuntimeError(STATE_ERROR_MSG) - - def update(self, *args: Any, **kwargs: Any) -> None: - raise RuntimeError(STATE_ERROR_MSG) - - def clear(self) -> None: - raise RuntimeError(STATE_ERROR_MSG) - - def pop(self, *args: Any, **kwargs: Any) -> None: - raise RuntimeError(STATE_ERROR_MSG) - - def popitem(self, *args: Any, **kwargs: Any) -> None: - raise RuntimeError(STATE_ERROR_MSG) - - def keys(self) -> List[str]: - return self._state_accessor.keys() - - def values(self) -> List[Any]: - """Values in the state dictionary. - - Note: This fetches all the values from the state. We - do not recommend using this method as it can be slow. - Consider accessing values directly via `state[key]`. - - Returns: - List[Any]: List of values in the state. - """ - - return self._state_accessor.values() - - def items(self) -> List[Tuple[str, Any]]: - """Items in the state dictionary. - - Note: This fetches all the key-value pairs from the state. - We do not recommend using this method as it can be slow. - If you need to iterate over the state, conditionally accessing - values, we recommend using the `keys` method instead and then - calling `state[key]` to access the value. - - Returns: - List[Tuple[str, Any]]: List of key-value pairs in the state. - """ - return self._state_accessor.items() - - def __iter__(self) -> Iterator[str]: - return self._state_accessor.__iter__() +# STATE_ERROR_MSG = ( +# "Cannot edit state directly. Use component update operations instead." +# ) + + +# class State: +# """Python class that stores state for a component instance. +# The instance id is stored in the `instance_id` attribute. + +# Example usage: + +# ```python +# from motion import Component + +# some_component = Component("SomeComponent") + +# @some_component.init_state +# def setUp(): +# return {"model": ...} + +# @some_component.serve("retrieve") +# def retrieve_nn(state, props): +# # model can be accessed via state["model"] +# prediction = state["model"](props["image_embedding"]) +# # match the prediction to some other data to do a retrieval +# nn_component_instance = SomeOtherMotionComponent(state.instance_id) +# return nn_component_instance.run("retrieve", props={"prediction": prediction}) + +# if __name__ == "__main__": +# c = some_component() +# nearest_neighbors = c.run("retrieve", props={"image_embedding": ...}) +# ``` +# """ + +# def __init__( +# self, +# component_name: str, +# instance_id: str, +# redis_host: str, +# redis_port: int, +# redis_db: int = 0, +# redis_password: Optional[str] = None, +# *args: Any, +# **kwargs: Any, +# ) -> None: +# self.component_name = component_name +# self._instance_id = instance_id +# self._state_accessor = StateAccessor( +# component_name, +# instance_id, +# 1000 +# * 60 +# * 2, # 2 minutes lock duration TODO: make this configurable +# redis_host, +# redis_port, +# redis_db, +# redis_password, +# ) +# super().__init__(*args, **kwargs) + +# def get_version(self) -> int: +# return self._state_accessor.version + +# @property +# def instance_id(self) -> str: +# """ +# Returns the instance id of the component. +# Useful if wanting to create other component instances +# within a serve or update operation. +# """ +# return self._instance_id + +# def clear_cache(self) -> None: +# # Clear the cache +# self._state_accessor.clear_cache() + +# def __getitem__(self, key: str) -> object: +# try: +# # Get from state accessor +# return self._state_accessor.get(key) +# except KeyError: +# raise KeyError( +# f"Key `{key}` not found in state for " +# + f"instance {self.component_name}__{self._instance_id}." +# ) + +# def get(self, key: str, default: Optional[Any] = None) -> Any: +# try: +# return self[key] +# except KeyError: +# return default + +# def __setitem__(self, key: str, value: Any) -> None: +# # Disable this functionality +# raise RuntimeError(STATE_ERROR_MSG) + +# def flushUpdateDict( +# self, update_dict: dict, from_migration: bool = False +# ) -> None: +# self._state_accessor.bulk_set(update_dict, from_migration) + +# def __delitem__(self, key: str) -> None: +# raise RuntimeError(STATE_ERROR_MSG) + +# def update(self, *args: Any, **kwargs: Any) -> None: +# raise RuntimeError(STATE_ERROR_MSG) + +# def clear(self) -> None: +# raise RuntimeError(STATE_ERROR_MSG) + +# def pop(self, *args: Any, **kwargs: Any) -> None: +# raise RuntimeError(STATE_ERROR_MSG) + +# def popitem(self, *args: Any, **kwargs: Any) -> None: +# raise RuntimeError(STATE_ERROR_MSG) + +# def keys(self) -> List[str]: +# return self._state_accessor.keys() + +# def values(self) -> List[Any]: +# """Values in the state dictionary. + +# Note: This fetches all the values from the state. We +# do not recommend using this method as it can be slow. +# Consider accessing values directly via `state[key]`. + +# Returns: +# List[Any]: List of values in the state. +# """ + +# return self._state_accessor.values() + +# def items(self) -> List[Tuple[str, Any]]: +# """Items in the state dictionary. + +# Note: This fetches all the key-value pairs from the state. +# We do not recommend using this method as it can be slow. +# If you need to iterate over the state, conditionally accessing +# values, we recommend using the `keys` method instead and then +# calling `state[key]` to access the value. + +# Returns: +# List[Tuple[str, Any]]: List of key-value pairs in the state. +# """ +# return self._state_accessor.items() + +# def __iter__(self) -> Iterator[str]: +# return iter(self._state_accessor.keys()) class MDataFrame(pd.DataFrame): diff --git a/motionstate/src/lib.rs b/motionstate/src/lib.rs index 441130ff..f7941e4d 100644 --- a/motionstate/src/lib.rs +++ b/motionstate/src/lib.rs @@ -1,5 +1,8 @@ -pub mod state_value; -use state_value::StateValue; +// pub mod state_value; +// use state_value::StateValue; + +pub mod temp_value; +use temp_value::TempValue; use pyo3::exceptions; use pyo3::prelude::*; @@ -9,11 +12,6 @@ use redlock::RedLock; use std::collections::HashMap; use std::sync::Arc; -/* -TODO: -* Use a redis lock - */ - #[pyclass] pub struct StateAccessor { component_name: String, @@ -81,6 +79,8 @@ impl StateAccessor { } pub fn set(&mut self, py: Python, key: &str, value: &PyAny) -> PyResult<()> { + // Warning: This function does not check if the value is a TempValue. + // But it is also never called from the Python side, so it's fine. let mut con = self.client.get_connection().unwrap(); let serialized_data = Arc::new(serialize_value(py, value)?); @@ -166,7 +166,7 @@ impl StateAccessor { Ok(()) } - pub fn bulk_set(&mut self, py: Python, items: &PyDict) -> PyResult<()> { + pub fn bulk_set(&mut self, py: Python, items: &PyDict, from_migration: bool) -> PyResult<()> { let mut con = self.client.get_connection().unwrap(); // Preserialize all the data @@ -176,53 +176,84 @@ impl StateAccessor { "MOTION_STATE:{}__{}/{}", self.component_name, self.instance_id, key ); - let serialized_data = Arc::new(serialize_value(py, value)?); - serialized_items.push((keyname, serialized_data)); + + // If value is of type TempValue, we should serialize + // the value inside it instead of the TempValue itself + // and extract the TTL from the TempValue. On default, + // the TTL will be None. + // let (value_to_serialize, ttl): (PyObject, Option); + if value.is_instance_of::() { + let temp_value: PyRef = value.extract()?; + // let value_to_serialize = &temp_value.value; + let value_ref: &PyAny = temp_value.value.as_ref(py); + let ttl = Some(temp_value.ttl); + + let serialized_data = Arc::new(serialize_value(py, value_ref)?); + serialized_items.push((keyname, serialized_data, ttl)); + } else { + let serialized_data = Arc::new(serialize_value(py, value)?); + serialized_items.push((keyname, serialized_data, None)); + } + + // let serialized_data = Arc::new(serialize_value(py, value_to_serialize)?); + // serialized_items.push((keyname, serialized_data, ttl)); } let mut pipeline = redis::pipe(); pipeline.atomic(); - // Acquire the lock using rslock + // If not from_migration, acquire the lock using rslock // Lockname will be MOTION_LOCK:__ - let lock_name = format!("MOTION_LOCK:{}__{}", self.component_name, self.instance_id); let mut lock = None; - - // Loop until we get the lock - for _ in 0..self.max_lock_attempts { - match self - .lock_manager - .lock(lock_name.as_bytes(), self.lock_duration) - { - Ok(Some(l)) => { - lock = Some(l); - break; - } - Ok(None) => { - // Lock was not acquired. Sleep for 100ms and try again. - std::thread::sleep(std::time::Duration::from_millis(100)); - } - Err(e) => { - // Handle the Redis error, maybe return an error or log it. - return Err(PyErr::new::(format!( - "Failed to acquire lock due to Redis error: {}", - e - ))); + if !from_migration { + let lock_name = format!("MOTION_LOCK:{}__{}", self.component_name, self.instance_id); + + // Loop until we get the lock + for _ in 0..self.max_lock_attempts { + match self + .lock_manager + .lock(lock_name.as_bytes(), self.lock_duration) + { + Ok(Some(l)) => { + lock = Some(l); + break; + } + Ok(None) => { + // Lock was not acquired. Sleep for 100ms and try again. + std::thread::sleep(std::time::Duration::from_millis(100)); + } + Err(e) => { + // Handle the Redis error, maybe return an error or log it. + return Err(PyErr::new::(format!( + "Failed to acquire lock due to Redis error: {}", + e + ))); + } } } - } - if lock.is_none() { - return Err(PyErr::new::(format!( - "Failed to acquire lock after {} attempts", - self.max_lock_attempts - ))); + if lock.is_none() { + return Err(PyErr::new::(format!( + "Failed to acquire lock after {} attempts", + self.max_lock_attempts + ))); + } } // Critical section - for (keyname, serialized_data) in serialized_items.iter() { + for (keyname, serialized_data, ttl) in serialized_items.iter() { // Insert the key and value into the cache self.cache.insert(keyname.clone(), serialized_data.clone()); - pipeline.cmd("SET").arg(keyname).arg(&**serialized_data); + + // If ttl is not None, set the TTL + if let Some(ttl) = ttl { + pipeline + .cmd("SETEX") + .arg(keyname) + .arg(ttl) + .arg(&**serialized_data); + } else { + pipeline.cmd("SET").arg(keyname).arg(&**serialized_data); + } } // Increment the version and write it to Redis @@ -249,8 +280,10 @@ impl StateAccessor { } self.version -= 1; - // Drop the lock - self.lock_manager.unlock(lock.as_ref().unwrap()); + // Drop the lock if from_migration is false + if !from_migration { + self.lock_manager.unlock(lock.as_ref().unwrap()); + } PyErr::new::(format!( "Redis bulk set error: {}", @@ -258,8 +291,10 @@ impl StateAccessor { )) })?; - // Drop the lock - self.lock_manager.unlock(lock.as_ref().unwrap()); + // Drop the lock if from_migration is false + if !from_migration { + self.lock_manager.unlock(lock.as_ref().unwrap()); + } Ok(()) } @@ -297,12 +332,6 @@ impl StateAccessor { } } - pub fn __iter__(&self, py: Python) -> PyResult { - let keys = self.keys(py)?; - let list = pyo3::types::PyList::new(py, &keys); - Ok(list.into()) - } - pub fn items(&mut self, py: Python) -> PyResult { let items_list = pyo3::types::PyList::empty(py); let pattern = format!( @@ -376,7 +405,7 @@ impl StateAccessor { // Serialization Helpers const MARKER_LIST: u8 = 0x01; const MARKER_DICT: u8 = 0x02; -const MARKER_STATE_VALUE: u8 = 0x03; +// const MARKER_STATE_VALUE: u8 = 0x03; fn cloudpickle_serialize(py: Python, value: &PyAny) -> PyResult> { let cloudpickle = py.import("cloudpickle")?; @@ -413,21 +442,31 @@ fn serialize_value(py: Python, value: &PyAny) -> PyResult> { serialized.extend(serialized_item); } Ok(serialized) - } else if value.is_instance_of::() { - let mut serialized = vec![MARKER_STATE_VALUE]; - - let saved_data = value.call_method0("save")?; - - // Check if the saved_data is bytes - let bytes_data = saved_data.downcast::().map_err(|_| { - PyErr::new::( - "'save' method must return a bytes object", - ) - })?; - - serialized.extend(bytes_data.as_bytes()); - Ok(serialized) - } else { + } + // else if value.is_instance_of::() { + // let mut serialized = vec![MARKER_STATE_VALUE]; + + // // Serialize the Python class's full name for deserialization purposes + // let class = value.getattr("__class__")?; + // let module_name = class.getattr("__module__")?.extract::()?; + // let class_name = class.getattr("__name__")?.extract::()?; + // let full_name = format!("{}.{}", module_name, class_name); + // serialized.extend((full_name.len() as u64).to_le_bytes().iter()); + // serialized.extend(full_name.as_bytes()); + + // let saved_data = value.call_method0("save")?; + + // // Check if the saved_data is bytes + // let bytes_data = saved_data.downcast::().map_err(|_| { + // PyErr::new::( + // "'save' method must return a bytes object", + // ) + // })?; + + // serialized.extend(bytes_data.as_bytes()); + // Ok(serialized) + // } + else { cloudpickle_serialize(py, value) } } @@ -469,13 +508,48 @@ fn deserialize_value(py: Python, value: &[u8]) -> PyResult { let mut cursor = 0; match value[cursor] { - MARKER_STATE_VALUE => { - cursor += 1; - let state_value_data = &value[cursor..]; - let state_value_type = py.get_type::(); - let result = state_value_type.call_method1("load", (state_value_data,))?; - Ok(result.into()) - } + // MARKER_STATE_VALUE => { + // cursor += 1; + + // // Deserialize the Python class's full name for deserialization purposes + // if let Some(class_name_bytes) = extract_next_slice(value, &mut cursor) { + // let full_name = std::str::from_utf8(class_name_bytes)?; + // let parts: Vec<&str> = full_name.split('.').collect(); + // if parts.len() != 2 { + // return Err(PyErr::new::(format!( + // "Invalid StateValue subclass name: {}", + // full_name + // ))); + // } + // let module_name = parts[0]; + // let class_name = parts[1]; + // let module = py.import(module_name).map_err(|e| { + // PyErr::new::(format!( + // "Failed to import module {}: {}", + // module_name, e + // )) + // })?; + // let class = module.getattr(class_name).map_err(|e| { + // PyErr::new::(format!( + // "Failed to get class {} from module {}: {}", + // class_name, module_name, e + // )) + // })?; + + // let state_value_data = &value[cursor..]; + // let result = class.call_method1("load", (state_value_data,))?; + // Ok(result.into()) + // } else { + // Err(PyErr::new::( + // "Failed to deserialize StateValue", + // )) + // } + + // // let state_value_data = &value[cursor..]; + // // let state_value_type = py.get_type::(); + // // let result = state_value_type.call_method1("load", (state_value_data,))?; + // // Ok(result.into()) + // } MARKER_LIST => { cursor += 1; let list = pyo3::types::PyList::empty(py); @@ -516,7 +590,8 @@ fn deserialize_value(py: Python, value: &[u8]) -> PyResult { #[pymodule] fn motionstate(_py: Python, m: &PyModule) -> PyResult<()> { m.add_class::()?; - m.add_class::()?; + // m.add_class::()?; + m.add_class::()?; Ok(()) } diff --git a/motionstate/src/state_value.rs b/motionstate/src/state_value.rs index d20f1fbe..ee5bb1d9 100644 --- a/motionstate/src/state_value.rs +++ b/motionstate/src/state_value.rs @@ -1,12 +1,19 @@ +// This file is not used yet. It is a placeholder for a future feature. + use pyo3::exceptions::PyNotImplementedError; use pyo3::prelude::*; use pyo3::types::{PyBytes, PyType}; -#[pyclass] +#[pyclass(subclass)] pub struct StateValue; #[pymethods] impl StateValue { + #[new] + pub fn new() -> Self { + StateValue {} + } + #[classmethod] pub fn load(_cls: &PyType, _data: &PyBytes) -> PyResult<()> { Err(PyNotImplementedError::new_err( diff --git a/motionstate/src/temp_value.rs b/motionstate/src/temp_value.rs new file mode 100644 index 00000000..de7bb3f0 --- /dev/null +++ b/motionstate/src/temp_value.rs @@ -0,0 +1,54 @@ +use pyo3::prelude::*; +use pyo3::PyObject; + +#[pyclass] +pub struct TempValue { + pub value: PyObject, + pub ttl: u64, +} + +#[pymethods] +impl TempValue { + #[new] + pub fn new(py: Python, value: PyObject, ttl: u64) -> Self { + TempValue { + value: value.into_py(py), + ttl, + } + } + + #[getter] + pub fn value(&self, py: Python) -> PyObject { + self.value.clone_ref(py) + } + + #[getter] + pub fn ttl(&self) -> u64 { + self.ttl + } + + #[setter] + pub fn set_ttl(&mut self, new_ttl: u64) { + self.ttl = new_ttl; + } +} + +#[cfg(test)] +mod tests { + use super::*; + use pyo3::types::IntoPyDict; + + #[test] + fn test_tempvalue_creation() { + let gil = Python::acquire_gil(); + let py = gil.python(); + let d = [("TempValue", py.get_type::())].into_py_dict(py); + let instance: PyObject = py + .eval("TempValue(value='hello', ttl=100)", Some(d), None) + .unwrap() + .extract() + .unwrap(); + let tempvalue: &TempValue = instance.extract(py).unwrap(); + assert_eq!(tempvalue.ttl, 100); + } +} diff --git a/tests/state/test_temp_value.py b/tests/state/test_temp_value.py new file mode 100644 index 00000000..7e1adbbb --- /dev/null +++ b/tests/state/test_temp_value.py @@ -0,0 +1,29 @@ +from motion import Component +from motion.state import TempValue + +import pytest +import time + +TempCounter = Component("TempCounter") + + +def test_temp_state_value(): + counter = TempCounter() + + # Assert nothing in it + with pytest.raises(KeyError): + counter.read_state("value") + + # Add a temp value + val = TempValue(0, ttl=1) + counter.write_state({"value": val}) + + # Check that it's there after clearing cache + assert counter.read_state("value", force_refresh=True) == 0 + + # Sleep for a bit + time.sleep(1) + + # Check that it's gone after clearing cache + with pytest.raises(KeyError): + counter.read_state("value", force_refresh=True) From 0d0e79cc80ca582161bd7917aea278702df266cb Mon Sep 17 00:00:00 2001 From: Shreya Shankar Date: Sun, 20 Aug 2023 13:21:29 -0700 Subject: [PATCH 08/37] Using serde for serialization --- motionstate/Cargo.toml | 2 + motionstate/src/lib.rs | 226 ++++++++++++++--------------------------- 2 files changed, 76 insertions(+), 152 deletions(-) diff --git a/motionstate/Cargo.toml b/motionstate/Cargo.toml index b07b0571..dc90ff81 100644 --- a/motionstate/Cargo.toml +++ b/motionstate/Cargo.toml @@ -12,3 +12,5 @@ crate-type = ["cdylib"] redis = "0.23" pyo3 = { version = "0.19", features = ["extension-module"] } redlock = "2.0.0" +serde = {version="1.0.183", features = ["derive"]} +bincode = "1.3.3" diff --git a/motionstate/src/lib.rs b/motionstate/src/lib.rs index f7941e4d..51ceed05 100644 --- a/motionstate/src/lib.rs +++ b/motionstate/src/lib.rs @@ -12,6 +12,18 @@ use redlock::RedLock; use std::collections::HashMap; use std::sync::Arc; +use serde::{Deserialize, Serialize}; + +#[derive(Debug, Serialize, Deserialize)] +enum PyValue { + Int(i64), + Float(f64), + String(String), + List(Vec), + Dict(HashMap), + // ... Add other types as needed. +} + #[pyclass] pub struct StateAccessor { component_name: String, @@ -403,9 +415,6 @@ impl StateAccessor { } // Serialization Helpers -const MARKER_LIST: u8 = 0x01; -const MARKER_DICT: u8 = 0x02; -// const MARKER_STATE_VALUE: u8 = 0x03; fn cloudpickle_serialize(py: Python, value: &PyAny) -> PyResult> { let cloudpickle = py.import("cloudpickle")?; @@ -423,166 +432,79 @@ fn cloudpickle_deserialize(py: Python, value: &[u8]) -> PyResult { Ok(obj.into()) } -fn serialize_value(py: Python, value: &PyAny) -> PyResult> { - if value.is_instance_of::() - || value.is_instance_of::() - || value.is_instance_of::() - { - Ok(value.str()?.to_string().into_bytes()) - } else if value.is_instance_of::() { - let mut serialized = vec![MARKER_DICT]; - serialized.extend(serialize_dict(py, value)?); - Ok(serialized) - } else if value.is_instance_of::() { - let list = value.downcast::()?; - let mut serialized = vec![MARKER_LIST]; - for item in list.iter() { - let serialized_item = serialize_value(py, item)?; - serialized.extend((serialized_item.len() as u64).to_le_bytes().iter()); - serialized.extend(serialized_item); +fn py_to_rust(value: &PyAny) -> PyResult { + if let Ok(val) = value.extract::() { + Ok(PyValue::Int(val)) + } else if let Ok(val) = value.extract::() { + Ok(PyValue::Float(val)) + } else if let Ok(val) = value.extract::() { + Ok(PyValue::String(val)) + } else if let Ok(val) = value.downcast::() { + let list: Vec<_> = val + .iter() + .map(|item| py_to_rust(item)) + .collect::>()?; + Ok(PyValue::List(list)) + } else if let Ok(val) = value.downcast::() { + let mut dict = HashMap::new(); + for (key, val) in val.iter() { + let key_str = key.extract::()?; + let val_rust = py_to_rust(val)?; + dict.insert(key_str, val_rust); } - Ok(serialized) - } - // else if value.is_instance_of::() { - // let mut serialized = vec![MARKER_STATE_VALUE]; - - // // Serialize the Python class's full name for deserialization purposes - // let class = value.getattr("__class__")?; - // let module_name = class.getattr("__module__")?.extract::()?; - // let class_name = class.getattr("__name__")?.extract::()?; - // let full_name = format!("{}.{}", module_name, class_name); - // serialized.extend((full_name.len() as u64).to_le_bytes().iter()); - // serialized.extend(full_name.as_bytes()); - - // let saved_data = value.call_method0("save")?; - - // // Check if the saved_data is bytes - // let bytes_data = saved_data.downcast::().map_err(|_| { - // PyErr::new::( - // "'save' method must return a bytes object", - // ) - // })?; - - // serialized.extend(bytes_data.as_bytes()); - // Ok(serialized) - // } - else { - cloudpickle_serialize(py, value) + Ok(PyValue::Dict(dict)) + } else { + Err(PyErr::new::( + "Unsupported type for bincode serialization", + )) } } -fn serialize_dict(py: Python, value: &PyAny) -> PyResult> { - let dict = value.downcast::()?; - let mut serialized = Vec::new(); - - for (key, val) in dict { - let key_bytes = serialize_value(py, key)?; - let val_bytes = serialize_value(py, val)?; - serialized.extend((key_bytes.len() as u64).to_le_bytes().iter()); - serialized.extend(key_bytes); - serialized.extend((val_bytes.len() as u64).to_le_bytes().iter()); - serialized.extend(val_bytes); +fn rust_to_py(py: Python, value: &PyValue) -> PyResult { + match value { + PyValue::Int(val) => Ok(val.into_py(py)), + PyValue::Float(val) => Ok(val.into_py(py)), + PyValue::String(val) => Ok(val.into_py(py)), + PyValue::List(val) => { + let list = PyList::empty(py); + for item in val { + let py_item = rust_to_py(py, item)?; + list.append(py_item)?; + } + Ok(list.into()) + } + PyValue::Dict(val) => { + let dict = PyDict::new(py); + for (key, value) in val { + let py_val = rust_to_py(py, value)?; + dict.set_item(key, py_val)?; + } + Ok(dict.into()) + } // ... Handle other cases. } - - Ok(serialized) } -fn extract_next_slice<'a>(value: &'a [u8], cursor: &mut usize) -> Option<&'a [u8]> { - if *cursor + 8 <= value.len() { - let len = u64::from_le_bytes(value[*cursor..*cursor + 8].try_into().unwrap()) as usize; - *cursor += 8; - if *cursor + len <= value.len() { - let result = &value[*cursor..*cursor + len]; - *cursor += len; - return Some(result); - } +fn serialize_value(py: Python, value: &PyAny) -> PyResult> { + if let Ok(rust_value) = py_to_rust(value) { + let serialized = bincode::serialize(&rust_value) + .map_err(|e| PyErr::new::(format!("{}", e)))?; + + Ok(serialized) + } else { + // Fall back to cloudpickle if not any of the defined types + let serialized = cloudpickle_serialize(py, value)?; + + Ok(serialized) } - None } fn deserialize_value(py: Python, value: &[u8]) -> PyResult { - if value.is_empty() { - // Return empty string for empty data - return Ok("".to_object(py)); - } - - let mut cursor = 0; - match value[cursor] { - // MARKER_STATE_VALUE => { - // cursor += 1; - - // // Deserialize the Python class's full name for deserialization purposes - // if let Some(class_name_bytes) = extract_next_slice(value, &mut cursor) { - // let full_name = std::str::from_utf8(class_name_bytes)?; - // let parts: Vec<&str> = full_name.split('.').collect(); - // if parts.len() != 2 { - // return Err(PyErr::new::(format!( - // "Invalid StateValue subclass name: {}", - // full_name - // ))); - // } - // let module_name = parts[0]; - // let class_name = parts[1]; - // let module = py.import(module_name).map_err(|e| { - // PyErr::new::(format!( - // "Failed to import module {}: {}", - // module_name, e - // )) - // })?; - // let class = module.getattr(class_name).map_err(|e| { - // PyErr::new::(format!( - // "Failed to get class {} from module {}: {}", - // class_name, module_name, e - // )) - // })?; - - // let state_value_data = &value[cursor..]; - // let result = class.call_method1("load", (state_value_data,))?; - // Ok(result.into()) - // } else { - // Err(PyErr::new::( - // "Failed to deserialize StateValue", - // )) - // } - - // // let state_value_data = &value[cursor..]; - // // let state_value_type = py.get_type::(); - // // let result = state_value_type.call_method1("load", (state_value_data,))?; - // // Ok(result.into()) - // } - MARKER_LIST => { - cursor += 1; - let list = pyo3::types::PyList::empty(py); - while let Some(item_bytes) = extract_next_slice(value, &mut cursor) { - let item = deserialize_value(py, item_bytes)?; - list.append(item)?; - } - Ok(list.into()) - } - MARKER_DICT => { - cursor += 1; - let dict = PyDict::new(py); - while let (Some(key_bytes), Some(val_bytes)) = ( - extract_next_slice(value, &mut cursor), - extract_next_slice(value, &mut cursor), - ) { - let key = deserialize_value(py, key_bytes)?; - let val = deserialize_value(py, val_bytes)?; - dict.set_item(key, val)?; - } - Ok(dict.into()) - } - _ => { - if let Ok(decoded) = std::str::from_utf8(value) { - if let Ok(int_value) = decoded.parse::() { - return Ok(int_value.into_py(py)); - } else if let Ok(float_value) = decoded.parse::() { - return Ok(float_value.into_py(py)); - } - Ok(decoded.to_string().into_py(py)) - } else { - cloudpickle_deserialize(py, value) - } + match bincode::deserialize::(value) { + Ok(rust_value) => rust_to_py(py, &rust_value), + Err(_) => { + // Fall back to pickle if bincode deserialization fails + let deserialized = cloudpickle_deserialize(py, value)?; + Ok(deserialized.into()) } } } From b1a8603bd8be3024b1383b1258f5450594a67412 Mon Sep 17 00:00:00 2001 From: Shreya Shankar Date: Sun, 20 Aug 2023 23:21:48 -0700 Subject: [PATCH 09/37] Switch to microsoft picologging --- motionstate/src/lib.rs | 2 +- poetry.lock | 64 ++++++++++++++++++++++++++++++++++++++++++ pyproject.toml | 2 +- 3 files changed, 66 insertions(+), 2 deletions(-) diff --git a/motionstate/src/lib.rs b/motionstate/src/lib.rs index 51ceed05..a3b36cf9 100644 --- a/motionstate/src/lib.rs +++ b/motionstate/src/lib.rs @@ -6,7 +6,7 @@ use temp_value::TempValue; use pyo3::exceptions; use pyo3::prelude::*; -use pyo3::types::{PyAny, PyBytes, PyDict, PyFloat, PyInt, PyList, PyString}; +use pyo3::types::{PyAny, PyBytes, PyDict, PyList}; use redis::Commands; use redlock::RedLock; use std::collections::HashMap; diff --git a/poetry.lock b/poetry.lock index 3d1a301d..6369f40e 100644 --- a/poetry.lock +++ b/poetry.lock @@ -1633,7 +1633,11 @@ setuptools = "*" name = "numpy" version = "1.25.0" description = "Fundamental package for array computing in Python" +<<<<<<< HEAD category = "main" +======= +category = "dev" +>>>>>>> 813cf91 (Switch to microsoft picologging) optional = false python-versions = ">=3.9" files = [ @@ -1753,6 +1757,54 @@ files = [ [package.dependencies] ptyprocess = ">=0.5" +[[package]] +name = "picologging" +version = "0.9.2" +description = "A fast and lightweight logging library for Python" +category = "main" +optional = false +python-versions = ">=3.7" +files = [ + {file = "picologging-0.9.2-cp310-cp310-macosx_10_15_universal2.whl", hash = "sha256:3ef4c40dd5029660d54949422eae1af4e1aa37f5fc2d551ccfc7bf31a72c4083"}, + {file = "picologging-0.9.2-cp310-cp310-macosx_10_15_x86_64.whl", hash = "sha256:d56134545a322e9bb97c95e52076dc86ccd4c9aa4ea21167f5f1c55f390de8de"}, + {file = "picologging-0.9.2-cp310-cp310-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:d3b59501f10cc088fe6a094ce3080ca52c538038081b2d3e632801ccd9a97e17"}, + {file = "picologging-0.9.2-cp310-cp310-manylinux_2_17_i686.manylinux2014_i686.whl", hash = "sha256:37b54895e2a122d0a009b82bd586a35a5762c66045958d0fc1d8c35a623ebc15"}, + {file = "picologging-0.9.2-cp310-cp310-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:f1b4ed6f6d574760e7ce8a277a0478b55da791b1251a6fc44ba2161b096f23ae"}, + {file = "picologging-0.9.2-cp310-cp310-win32.whl", hash = "sha256:8abb06d75563f35f69aef1fa59753705e2e566a97f954694f7b445a1f638fef7"}, + {file = "picologging-0.9.2-cp310-cp310-win_amd64.whl", hash = "sha256:eeae1f52c1a6aeb88256f484d6ae52e92aa4d5761e66c34a913fc98226e269f5"}, + {file = "picologging-0.9.2-cp311-cp311-macosx_10_15_universal2.whl", hash = "sha256:b62fdb47ae7c1261e945e0f562d0962b8559e1fd46afc1be30f7ca7decc01eca"}, + {file = "picologging-0.9.2-cp311-cp311-macosx_10_15_x86_64.whl", hash = "sha256:4fe519f607d03ccb0a629eb23491a9e155b8945e08b8a153600b2853852d9eda"}, + {file = "picologging-0.9.2-cp311-cp311-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:7dfff150c3cf081e4a68c2528687cb7de8114f48578eaa07fb6168e7de2f012f"}, + {file = "picologging-0.9.2-cp311-cp311-manylinux_2_17_i686.manylinux2014_i686.whl", hash = "sha256:bb5a85e87bce59a1874494ae2995f1fa4bcbf139e753af717a768313d6057404"}, + {file = "picologging-0.9.2-cp311-cp311-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:d45472e29e40be5b1b473cfa4c320adebb088fdb7bd47f9111d21ee37b3fdd33"}, + {file = "picologging-0.9.2-cp311-cp311-win32.whl", hash = "sha256:8af05accda1babea4804c65262c7ac1e51586290f7fbff6e7fe93dca09ca1b0f"}, + {file = "picologging-0.9.2-cp311-cp311-win_amd64.whl", hash = "sha256:b46ac219edac40d98f891e6a2064dbf10f1af328d6e6038997ebaf8b34c8de4f"}, + {file = "picologging-0.9.2-cp37-cp37m-macosx_10_15_x86_64.whl", hash = "sha256:787f7b0ae666500d99836cd7704d58f752c5f60fd2c2f0c86ac00c09bfe18b80"}, + {file = "picologging-0.9.2-cp37-cp37m-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:01f35a6a3a0bbfaca8cd8a15af94d6cea1fe8a75d3295aa3779b1e048596fcc1"}, + {file = "picologging-0.9.2-cp37-cp37m-manylinux_2_17_i686.manylinux2014_i686.whl", hash = "sha256:79a2117cd911aaaf705ef15510ca68478666e3d9f8840f108270b7650ad6d235"}, + {file = "picologging-0.9.2-cp37-cp37m-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:e2d6886efbd31cb0fe1b900320056f3ad6b785029c8d2a1d64b1ab0b6e59164c"}, + {file = "picologging-0.9.2-cp37-cp37m-win32.whl", hash = "sha256:39fcb164b6ce296ad6598397f2f8a21e71f84f42a73cfc08963d227f2ece17db"}, + {file = "picologging-0.9.2-cp37-cp37m-win_amd64.whl", hash = "sha256:fc7c23960a2095cb3d57a60e3dfead16cfde99adcb635efeeb05a09bd5b28b20"}, + {file = "picologging-0.9.2-cp38-cp38-macosx_10_15_universal2.whl", hash = "sha256:a98e5b2516763943fab7b5a29a3a1a387a69b1a5ba7e2c145a9f4221ee6ae6b5"}, + {file = "picologging-0.9.2-cp38-cp38-macosx_10_15_x86_64.whl", hash = "sha256:872a21939523f6fb72026d6a6e09a72df21221b922c62919e129cb69565506fa"}, + {file = "picologging-0.9.2-cp38-cp38-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:47c13801574aa1eb89297a870d47b59dd18f5b5ad24b30d96ae8ba438439b604"}, + {file = "picologging-0.9.2-cp38-cp38-manylinux_2_17_i686.manylinux2014_i686.whl", hash = "sha256:dc4d9053113670a11bf2f69a8a69cca88d57eda7b4e3b1c0eb1a8cf88d49fa72"}, + {file = "picologging-0.9.2-cp38-cp38-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:98787d9dc4401672d0c1f39af1095fc4f53db3b07e2e17fb74059778939ab32d"}, + {file = "picologging-0.9.2-cp38-cp38-win32.whl", hash = "sha256:e322d1f5b6b7b9f2023cd205cd2ce5749e944359d5f53928e3c3d850f910aa33"}, + {file = "picologging-0.9.2-cp38-cp38-win_amd64.whl", hash = "sha256:a8ab594d621104d7618929663a5b03af4b2a58679b594f19ba887f4e5d79fb1c"}, + {file = "picologging-0.9.2-cp39-cp39-macosx_10_15_universal2.whl", hash = "sha256:9d7c02d0fc015dabde3c096d7ce84bd7f167114abba37c7dd6b761086518b6fa"}, + {file = "picologging-0.9.2-cp39-cp39-macosx_10_15_x86_64.whl", hash = "sha256:eefcc1e5bdc0003b37973e803c80b430cd3c1a9ab53839d3e5562d890be1cf77"}, + {file = "picologging-0.9.2-cp39-cp39-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:c41ee46960d581faa2fe20098cd0541701d35c04c2e89774781524e7290cd897"}, + {file = "picologging-0.9.2-cp39-cp39-manylinux_2_17_i686.manylinux2014_i686.whl", hash = "sha256:37e61e6770113473e0aee841b14b6a92e3c8e1982a038f141e5dcd88ac7ff446"}, + {file = "picologging-0.9.2-cp39-cp39-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:3aa728c45ef76c284b9febb794998e784ddf8e508d460e9d5f1ac70c0303317f"}, + {file = "picologging-0.9.2-cp39-cp39-win32.whl", hash = "sha256:ab1bbb809ab955ca852dc8a3e78fe14090280dcd81337f8efcb38272b3f24291"}, + {file = "picologging-0.9.2-cp39-cp39-win_amd64.whl", hash = "sha256:bb026157fd752be9388d9df134631a5089f83e2dbeb1bd85319e5eb4ed410964"}, + {file = "picologging-0.9.2.tar.gz", hash = "sha256:bcb578063a2e2af01948b5d1cbd08c1d54a5411c916da826bf3f695724b93623"}, +] + +[package.extras] +dev = ["black", "flaky", "hypothesis", "pre-commit", "pytest", "pytest-cov", "rich"] + [[package]] name = "pkginfo" version = "1.9.6" @@ -2273,7 +2325,11 @@ pytest = ">=5.0.0" name = "python-dateutil" version = "2.8.2" description = "Extensions to the standard Python datetime module" +<<<<<<< HEAD category = "main" +======= +category = "dev" +>>>>>>> 813cf91 (Switch to microsoft picologging) optional = false python-versions = "!=3.0.*,!=3.1.*,!=3.2.*,>=2.7" files = [ @@ -2849,7 +2905,11 @@ files = [ name = "six" version = "1.16.0" description = "Python 2 and 3 compatibility utilities" +<<<<<<< HEAD category = "main" +======= +category = "dev" +>>>>>>> 813cf91 (Switch to microsoft picologging) optional = false python-versions = ">=2.7, !=3.0.*, !=3.1.*, !=3.2.*" files = [ @@ -3352,4 +3412,8 @@ testing = ["big-O", "flake8 (<5)", "jaraco.functools", "jaraco.itertools", "more [metadata] lock-version = "2.0" python-versions = "^3.9" +<<<<<<< HEAD content-hash = "4140747a87459b88f9e7727e41937062503dda3b657e625b7f5a2e8c6c81865c" +======= +content-hash = "09d00ee8cb977733834c58724d20bf7601093113cb47b5d057aa805c98d42c56" +>>>>>>> 813cf91 (Switch to microsoft picologging) diff --git a/pyproject.toml b/pyproject.toml index 7eb8de2c..60a2d2d4 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -9,7 +9,6 @@ packages = [{include = "motion"}] [tool.poetry.dependencies] python = "^3.9" click = "^8.1.3" -colorlog = "^6.7.0" pydantic = "^1.10.7" cloudpickle = "^2.0" redis = "^4.5.5" @@ -19,6 +18,7 @@ pyyaml = "^6.0.1" pyarrow = "^13.0.0" pandas = "^2.1.0" tqdm = "^4.66.1" +picologging = "^0.9.2" [tool.poetry.group.dev.dependencies] poetry-types = "^0.3.5" From 2d09532d265e1736c664cd089c987f1f0fc8ebc0 Mon Sep 17 00:00:00 2001 From: Shreya Shankar Date: Thu, 23 Nov 2023 09:18:54 -0600 Subject: [PATCH 10/37] Make sure tests pass --- .gitignore | 5 +++++ motion/utils.py | 4 +++- motionstate/src/lib.rs | 5 ++++- motionstate/src/temp_value.rs | 23 +++++++++++++---------- tests/parallel/test_async.py | 5 ++--- 5 files changed, 27 insertions(+), 15 deletions(-) diff --git a/.gitignore b/.gitignore index b95a71e8..9cc263f1 100644 --- a/.gitignore +++ b/.gitignore @@ -27,4 +27,9 @@ motionstate/target* motionstate/Cargo.lock *rustc* motionstate*.so +<<<<<<< HEAD >>>>>>> e4aede0 (Successfully imported bare bones library version) +======= +*.whl +unnecessary* +>>>>>>> c994961 (Make sure tests pass) diff --git a/motion/utils.py b/motion/utils.py index 0a3252b1..a386d781 100644 --- a/motion/utils.py +++ b/motion/utils.py @@ -159,13 +159,15 @@ def clear_instance(instance_name: str) -> bool: return False # Delete the instance state, version, and cached results - redis_con.delete(f"MOTION_STATE:{instance_name}") redis_con.delete(f"MOTION_VERSION:{instance_name}") redis_con.delete(f"MOTION_LOCK:{instance_name}") + state_vals_to_delete = redis_con.keys(f"MOTION_STATE:{instance_name}/*") results_to_delete = redis_con.keys(f"MOTION_RESULT:{instance_name}/*") queues_to_delete = redis_con.keys(f"MOTION_QUEUE:{instance_name}/*") pipeline = redis_con.pipeline() + for state_val in state_vals_to_delete: + pipeline.delete(state_val) for result in results_to_delete: pipeline.delete(result) for queue in queues_to_delete: diff --git a/motionstate/src/lib.rs b/motionstate/src/lib.rs index a3b36cf9..37b5620c 100644 --- a/motionstate/src/lib.rs +++ b/motionstate/src/lib.rs @@ -527,6 +527,7 @@ mod tests { let _state = StateAccessor::new( "component".to_string(), "instance".to_string(), + 180 as u64, "127.0.0.1", 6381, 0, @@ -540,6 +541,7 @@ mod tests { let result = StateAccessor::new( "component".to_string(), "instance".to_string(), + 180 as u64, "invalid", 6381, 0, @@ -554,6 +556,7 @@ mod tests { let mut state = StateAccessor::new( "component".to_string(), "instance".to_string(), + 180 as u64, "127.0.0.1", 6381, 0, @@ -563,7 +566,7 @@ mod tests { // Set a value to Redis let _ = state - .bulk_set(py, [("test_key", 42)].into_py_dict(py)) + .bulk_set(py, [("test_key", 42)].into_py_dict(py), false) .unwrap(); // Clear cache to simulate fetching from Redis diff --git a/motionstate/src/temp_value.rs b/motionstate/src/temp_value.rs index de7bb3f0..68789cb6 100644 --- a/motionstate/src/temp_value.rs +++ b/motionstate/src/temp_value.rs @@ -40,15 +40,18 @@ mod tests { #[test] fn test_tempvalue_creation() { - let gil = Python::acquire_gil(); - let py = gil.python(); - let d = [("TempValue", py.get_type::())].into_py_dict(py); - let instance: PyObject = py - .eval("TempValue(value='hello', ttl=100)", Some(d), None) - .unwrap() - .extract() - .unwrap(); - let tempvalue: &TempValue = instance.extract(py).unwrap(); - assert_eq!(tempvalue.ttl, 100); + Python::with_gil(|py| { + // Your code that requires Python's GIL goes here + let d = [("TempValue", py.get_type::())].into_py_dict(py); + let instance: PyObject = py + .eval("TempValue(value='hello', ttl=100)", Some(d), None) + .unwrap() + .extract() + .unwrap(); + + // Extract ttl as i64 and compare + let ttl_value: i64 = instance.getattr(py, "ttl").unwrap().extract(py).unwrap(); + assert_eq!(ttl_value, 100); + }); } } diff --git a/tests/parallel/test_async.py b/tests/parallel/test_async.py index 3ebb6ad9..f93e4224 100644 --- a/tests/parallel/test_async.py +++ b/tests/parallel/test_async.py @@ -50,13 +50,12 @@ async def test_async_update(): @pytest.mark.asyncio -@pytest.mark.timeout(1) # This test should take less than 3 seconds +@pytest.mark.timeout(2) # This test should take less than 3 seconds async def test_gather(): c = Counter(disable_update_task=True) tasks = [ - c.arun("multiply", props={"value": i}, flush_update=True) - for i in range(100) + c.arun("multiply", props={"value": i}, flush_update=True) for i in range(100) ] # Run all tasks at the same time await asyncio.gather(*tasks) From 05649f0bc5d9d25a6d079c806d39411b898f4fdc Mon Sep 17 00:00:00 2001 From: Shreya Shankar Date: Fri, 24 Nov 2023 10:51:31 -0600 Subject: [PATCH 11/37] Make sure tests pass --- motion/__init__.py | 3 --- motion/execute.py | 14 ++++++------- motion/instance.py | 19 ++++++++---------- motion/migrate.py | 3 +-- motion/server/update_task.py | 8 ++++---- motion/state/state.py | 3 +-- poetry.lock | 36 +--------------------------------- tests/state/test_db_conn.py | 16 +++++++-------- tests/state/test_temp_value.py | 9 +++++---- 9 files changed, 34 insertions(+), 77 deletions(-) diff --git a/motion/__init__.py b/motion/__init__.py index 0162211a..cd3ed39a 100644 --- a/motion/__init__.py +++ b/motion/__init__.py @@ -11,7 +11,6 @@ from motion.dicts import MDataFrame from motion.copy_utils import copy_db from motion.server.application import Application -from motionstate import StateValue, State __all__ = [ "Component", @@ -25,6 +24,4 @@ "copy_db", "RedisParams", "Application", - "StateValue", - "State", ] diff --git a/motion/execute.py b/motion/execute.py index e83ba577..cd9d1009 100644 --- a/motion/execute.py +++ b/motion/execute.py @@ -134,9 +134,11 @@ def _connectToRedis(self) -> Tuple[RedisParams, redis.Redis]: r = redis.Redis(**param_dict) return rp, r - def _loadState(self) -> State: + def _loadState(self, force_refresh: bool = True) -> State: # Clear state cache - self._state.clear_cache() + if force_refresh: + self._state.clear_cache() + return self._state def setUp(self, **kwargs: Any) -> Dict[str, Any]: # Set up initial state @@ -263,9 +265,7 @@ def shutdown(self, is_open: bool) -> None: self.monitor_thread.join() - def _updateState( - self, new_state: Dict[str, Any], force_update: bool = True - ) -> None: + def _updateState(self, new_state: Dict[str, Any]) -> None: if not new_state: return @@ -319,7 +319,7 @@ def _enqueue_and_trigger_update( raise ValueError("State update must be a dict.") else: # Update state - self._updateState(state_update, force_update=False) + self._updateState(state_update) except Exception as e: raise RuntimeError( "Error running update route in main process: " + str(e) @@ -383,7 +383,7 @@ async def _async_enqueue_and_trigger_update( raise ValueError("State update must be a dict.") else: # Update state - self._updateState(state_update, force_update=False) + self._updateState(state_update) except Exception as e: raise RuntimeError( "Error running update route in main process: " + str(e) diff --git a/motion/instance.py b/motion/instance.py index 67fe028e..ee64ede9 100644 --- a/motion/instance.py +++ b/motion/instance.py @@ -165,7 +165,7 @@ def setUp(): """ return self._executor._state.get_version() # type: ignore - def write_state(self, state_update: Dict[str, Any], latest: bool = False) -> None: + def write_state(self, state_update: Dict[str, Any]) -> None: """Writes the state update to the component instance's state. If a update op is currently running, the state update will be applied after the update op is finished. Warning: this could @@ -195,17 +195,12 @@ def setUp(): Args: state_update (Dict[str, Any]): Dictionary of key-value pairs to update the state with. - latest (bool, optional): Whether or not to apply the update - to the latest version of the state. - If true, Motion will redownload the latest version - of the state and apply the update to that version. You - only need to set this to true if you are updating an - instance you connected to a while ago and might be - outdated. Defaults to False. """ - self._executor._updateState(state_update, force_update=latest) + self._executor._updateState(state_update) - def read_state(self, key: str, default_value: Optional[Any] = None) -> Any: + def read_state( + self, key: str, default_value: Optional[Any] = None, force_refresh: bool = True + ) -> Any: """Gets the current value for the key in the component instance's state. Usage: @@ -233,12 +228,14 @@ def setUp(): key (str): Key in the state to get the value for. default_value (Optional[Any], optional): Default value to return if the key is not found. Defaults to None. + force_refresh (bool, optional): Read the latest value of the state + in the KV store, otherwise return what is in the cache. Returns: Any: Current value for the key, or default_value if the key is not found. """ - return self._executor._loadState().get(key, default_value) + return self._executor._loadState(force_refresh).get(key, default_value) def flush_update(self, dataflow_key: str) -> None: """Flushes the update queue corresponding to the dataflow diff --git a/motion/migrate.py b/motion/migrate.py index 1f6e638c..fd14d545 100644 --- a/motion/migrate.py +++ b/motion/migrate.py @@ -148,8 +148,7 @@ def migrate( ( instance_name, self.migrate_func, - self.component._load_state_func, - self.component._save_state_func, + lock_timeout, ) for instance_name in instance_names ] diff --git a/motion/server/update_task.py b/motion/server/update_task.py index e1b97883..b9d2cabe 100644 --- a/motion/server/update_task.py +++ b/motion/server/update_task.py @@ -101,10 +101,10 @@ def custom_run(self) -> None: old_state = State( self.instance_name.split("__")[0], self.instance_name.split("__")[1], - redis_host=self.redis_host, - redis_port=self.redis_port, - redis_db=self.redis_db, - redis_password=self.redis_password, + redis_host=self.redis_params["host"], + redis_port=self.redis_params["port"], + redis_db=self.redis_params["db"], + redis_password=self.redis_params["password"], ) state_update = self.routes[queue_name].run( diff --git a/motion/state/state.py b/motion/state/state.py index 52d6566f..a44286c4 100644 --- a/motion/state/state.py +++ b/motion/state/state.py @@ -1,6 +1,5 @@ """ -This file contains the props class, which is used to store -properties of a flow. +This file contains the state class. """ from typing import Any, Iterator, List, Optional, Tuple diff --git a/poetry.lock b/poetry.lock index 6369f40e..ffe4c4af 100644 --- a/poetry.lock +++ b/poetry.lock @@ -391,24 +391,6 @@ files = [ {file = "colorama-0.4.6.tar.gz", hash = "sha256:08695f5cb7ed6e0531a20572697297273c47b8cae5a63ffc6d6ed5c201be6e44"}, ] -[[package]] -name = "colorlog" -version = "6.7.0" -description = "Add colours to the output of Python's logging module." -category = "main" -optional = false -python-versions = ">=3.6" -files = [ - {file = "colorlog-6.7.0-py2.py3-none-any.whl", hash = "sha256:0d33ca236784a1ba3ff9c532d4964126d8a2c44f1f0cb1d2b0728196f512f662"}, - {file = "colorlog-6.7.0.tar.gz", hash = "sha256:bd94bd21c1e13fac7bd3153f4bc3a7dc0eb0974b8bc2fdf1a989e474f6e582e5"}, -] - -[package.dependencies] -colorama = {version = "*", markers = "sys_platform == \"win32\""} - -[package.extras] -development = ["black", "flake8", "mypy", "pytest", "types-colorama"] - [[package]] name = "coverage" version = "7.2.7" @@ -1633,11 +1615,7 @@ setuptools = "*" name = "numpy" version = "1.25.0" description = "Fundamental package for array computing in Python" -<<<<<<< HEAD category = "main" -======= -category = "dev" ->>>>>>> 813cf91 (Switch to microsoft picologging) optional = false python-versions = ">=3.9" files = [ @@ -2325,11 +2303,7 @@ pytest = ">=5.0.0" name = "python-dateutil" version = "2.8.2" description = "Extensions to the standard Python datetime module" -<<<<<<< HEAD category = "main" -======= -category = "dev" ->>>>>>> 813cf91 (Switch to microsoft picologging) optional = false python-versions = "!=3.0.*,!=3.1.*,!=3.2.*,>=2.7" files = [ @@ -2905,11 +2879,7 @@ files = [ name = "six" version = "1.16.0" description = "Python 2 and 3 compatibility utilities" -<<<<<<< HEAD category = "main" -======= -category = "dev" ->>>>>>> 813cf91 (Switch to microsoft picologging) optional = false python-versions = ">=2.7, !=3.0.*, !=3.1.*, !=3.2.*" files = [ @@ -3412,8 +3382,4 @@ testing = ["big-O", "flake8 (<5)", "jaraco.functools", "jaraco.itertools", "more [metadata] lock-version = "2.0" python-versions = "^3.9" -<<<<<<< HEAD -content-hash = "4140747a87459b88f9e7727e41937062503dda3b657e625b7f5a2e8c6c81865c" -======= -content-hash = "09d00ee8cb977733834c58724d20bf7601093113cb47b5d057aa805c98d42c56" ->>>>>>> 813cf91 (Switch to microsoft picologging) +content-hash = "7b3052cf6cfb6f45a3811cb455180b681b7b5706cf6f7bed4494a8f6548b40e7" diff --git a/tests/state/test_db_conn.py b/tests/state/test_db_conn.py index cf471a53..1945d947 100644 --- a/tests/state/test_db_conn.py +++ b/tests/state/test_db_conn.py @@ -14,6 +14,10 @@ def setUp(): path = ":file::memory:?cache=shared:" conn = sqlite3.connect(path) cursor = conn.cursor() + + # Drop table if exists + cursor.execute("DROP TABLE IF EXISTS users") + cursor.execute( """CREATE TABLE IF NOT EXISTS users (id INTEGER PRIMARY KEY AUTOINCREMENT, @@ -21,12 +25,8 @@ def setUp(): age INTEGER)""" ) - cursor.execute( - "INSERT INTO users (name, age) VALUES (?, ?)", ("John Doe", 25) - ) - cursor.execute( - "INSERT INTO users (name, age) VALUES (?, ?)", ("Jane Smith", 30) - ) + cursor.execute("INSERT INTO users (name, age) VALUES (?, ?)", ("John Doe", 25)) + cursor.execute("INSERT INTO users (name, age) VALUES (?, ?)", ("Jane Smith", 30)) conn.commit() return {"path": path, "fit_count": 0} @@ -51,9 +51,7 @@ def increment(state, props): def test_db_component(): c_instance = c() - assert c_instance.run("count", props={"value": 1}, flush_update=True) == [ - (2,) - ] + assert c_instance.run("count", props={"value": 1}, flush_update=True) == [(2,)] c_instance.run("something", props={"value": 1}, flush_update=True) assert c_instance.run("something", props={"value": 5}) == 1 diff --git a/tests/state/test_temp_value.py b/tests/state/test_temp_value.py index 7e1adbbb..25bce547 100644 --- a/tests/state/test_temp_value.py +++ b/tests/state/test_temp_value.py @@ -11,13 +11,15 @@ def test_temp_state_value(): counter = TempCounter() # Assert nothing in it - with pytest.raises(KeyError): - counter.read_state("value") + assert counter.read_state("value") is None # Add a temp value val = TempValue(0, ttl=1) counter.write_state({"value": val}) + # Check that it's there before clearing cache + assert counter.read_state("value") == 0 + # Check that it's there after clearing cache assert counter.read_state("value", force_refresh=True) == 0 @@ -25,5 +27,4 @@ def test_temp_state_value(): time.sleep(1) # Check that it's gone after clearing cache - with pytest.raises(KeyError): - counter.read_state("value", force_refresh=True) + assert counter.read_state("value", force_refresh=True) is None From 216a58c5b4a79c87865d2c9cdcdab6f62406f57e Mon Sep 17 00:00:00 2001 From: Shreya Shankar Date: Fri, 24 Nov 2023 10:55:49 -0600 Subject: [PATCH 12/37] Make mypy work --- motion/migrate.py | 3 +-- motion/state/__init__.py | 2 +- motion/state/state.py | 16 ++++++++-------- motion/utils.py | 3 ++- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/motion/migrate.py b/motion/migrate.py index fd14d545..12df502c 100644 --- a/motion/migrate.py +++ b/motion/migrate.py @@ -133,10 +133,9 @@ def migrate( ] if not instance_names: instance_names = [ - key.decode("utf-8").replace("MOTION_VERSION:", "") + key.decode("utf-8").replace("MOTION_VERSION:", "") # type: ignore for key in redis_con.keys(f"MOTION_VERSION:{self.component.name}__*") ] - print(instance_names) if not instance_names: logger.warning(f"No instances for component {self.component.name} found.") diff --git a/motion/state/__init__.py b/motion/state/__init__.py index 07a33b97..12769269 100644 --- a/motion/state/__init__.py +++ b/motion/state/__init__.py @@ -1,4 +1,4 @@ from motion.state.state import State -from motionstate import TempValue +from motionstate import TempValue # type: ignore __all__ = ["State", "TempValue"] diff --git a/motion/state/state.py b/motion/state/state.py index a44286c4..ccdc52b1 100644 --- a/motion/state/state.py +++ b/motion/state/state.py @@ -3,7 +3,7 @@ """ from typing import Any, Iterator, List, Optional, Tuple -from motionstate import StateAccessor +from motionstate import StateAccessor # type: ignore STATE_ERROR_MSG = "Cannot edit state directly. Use component update operations instead." @@ -62,7 +62,7 @@ def __init__( super().__init__(*args, **kwargs) def get_version(self) -> int: - return self._state_accessor.version + return self._state_accessor.version # type: ignore @property def instance_id(self) -> str: @@ -71,13 +71,13 @@ def instance_id(self) -> str: Useful if wanting to create other component instances within a serve or update operation. """ - return self._instance_id + return self._instance_id # type: ignore def clear_cache(self) -> None: # Clear the cache self._state_accessor.clear_cache() - def __getitem__(self, key: str) -> object: + def __getitem__(self, key: str) -> Any: try: # Get from state accessor return self._state_accessor.get(key) @@ -116,7 +116,7 @@ def popitem(self, *args: Any, **kwargs: Any) -> None: raise RuntimeError(STATE_ERROR_MSG) def keys(self) -> List[str]: - return self._state_accessor.keys() + return self._state_accessor.keys() # type: ignore def values(self) -> List[Any]: """Values in the state dictionary. @@ -129,7 +129,7 @@ def values(self) -> List[Any]: List[Any]: List of values in the state. """ - return self._state_accessor.values() + return self._state_accessor.values() # type: ignore def items(self) -> List[Tuple[str, Any]]: """Items in the state dictionary. @@ -143,7 +143,7 @@ def items(self) -> List[Tuple[str, Any]]: Returns: List[Tuple[str, Any]]: List of key-value pairs in the state. """ - return self._state_accessor.items() + return self._state_accessor.items() # type: ignore def __iter__(self) -> Iterator[str]: - return iter(self._state_accessor.keys()) + return iter(self._state_accessor.keys()) # type: ignore diff --git a/motion/utils.py b/motion/utils.py index a386d781..df464cce 100644 --- a/motion/utils.py +++ b/motion/utils.py @@ -254,7 +254,8 @@ def configureLogging(level: str) -> None: # "CRITICAL": "bold_red", # }, # ) - logging.basicConfig(level=level) + + logging.basicConfig(level=level) # type: ignore # logger = logging.getLogger("motion") # if logger.hasHandlers(): # logger.handlers.clear() From 2d481c2141ca60a4d65535f28e8a59e81e340a56 Mon Sep 17 00:00:00 2001 From: Shreya Shankar Date: Fri, 24 Nov 2023 17:11:21 -0600 Subject: [PATCH 13/37] Tests should pass --- .gitignore | 12 +- Makefile | 2 +- motion/copy_utils.py | 3 +- motion/execute.py | 12 +- motion/migrate.py | 5 +- motion/server/update_task.py | 5 +- motion/state/state.py | 16 +- motion/utils.py | 5 +- motionstate/src/lib.rs | 45 +++- tests/state/test_rust_vs_cloudpickle.py | 51 ++++ unnecessary.py | 334 ++++++++++++++++++++++++ 11 files changed, 438 insertions(+), 52 deletions(-) create mode 100644 tests/state/test_rust_vs_cloudpickle.py create mode 100644 unnecessary.py diff --git a/.gitignore b/.gitignore index 9cc263f1..142fe0bf 100644 --- a/.gitignore +++ b/.gitignore @@ -16,20 +16,12 @@ dist* site* *package-lock.json projects -<<<<<<< HEAD -unnecessary.py -motionstate* -*.whl -======= motionstate/debug* motionstate/target* motionstate/Cargo.lock *rustc* motionstate*.so -<<<<<<< HEAD ->>>>>>> e4aede0 (Successfully imported bare bones library version) -======= +*motionenv* *.whl -unnecessary* ->>>>>>> c994961 (Make sure tests pass) +*unnecessary* \ No newline at end of file diff --git a/Makefile b/Makefile index a16ea313..4ee0370a 100644 --- a/Makefile +++ b/Makefile @@ -23,7 +23,7 @@ build: cd motionstate && \ echo "Building motionstate" && \ maturin develop && \ - maturin build --release && \ + maturin build --release --interpreter python3.9 python3.10 python3.11 && \ python -m venv ../.motionenv && \ source ../.motionenv/bin/activate && pip install target/wheels/motionstate*.whl && deactivate && \ echo "Copying .so file to motion" && \ diff --git a/motion/copy_utils.py b/motion/copy_utils.py index 11bac16f..1fff6264 100644 --- a/motion/copy_utils.py +++ b/motion/copy_utils.py @@ -3,8 +3,7 @@ from one Redis instance to another. """ -import logging - +import picologging as logging import redis.asyncio as redis from motion.utils import RedisParams diff --git a/motion/execute.py b/motion/execute.py index cd9d1009..360f63e8 100644 --- a/motion/execute.py +++ b/motion/execute.py @@ -65,10 +65,7 @@ def __init__( self._state = State( self._instance_name.split("__")[0], self._instance_name.split("__")[1], - redis_host=self._redis_params.host, - redis_port=self._redis_params.port, - redis_db=self._redis_params.db, - redis_password=self._redis_params.password, + self._redis_params.dict(), ) # If it is version 0, then call the init_state_func if self._state.get_version() == 0 and self._init_state_func is not None: @@ -466,6 +463,13 @@ def run( # Run the serve route if key in self._serve_routes.keys(): + # Check if the function is an async function + if asyncio.iscoroutinefunction(self._serve_routes[key].udf): + raise TypeError( + f"Route {key} is an async function. " + + "Call `await instance.arun(...)` instead." + ) + route_hit = True ( route_run, diff --git a/motion/migrate.py b/motion/migrate.py index 12df502c..1aae7fb1 100644 --- a/motion/migrate.py +++ b/motion/migrate.py @@ -28,10 +28,7 @@ def process_migration( state = State( instance_name.split("__")[0], instance_name.split("__")[1], - redis_host=rp.host, - redis_port=rp.port, - redis_db=rp.db, - redis_password=rp.password, + redis_params=rp.dict(), ) # Acquire lock to prevent other writes during migration diff --git a/motion/server/update_task.py b/motion/server/update_task.py index b9d2cabe..02b27a87 100644 --- a/motion/server/update_task.py +++ b/motion/server/update_task.py @@ -101,10 +101,7 @@ def custom_run(self) -> None: old_state = State( self.instance_name.split("__")[0], self.instance_name.split("__")[1], - redis_host=self.redis_params["host"], - redis_port=self.redis_params["port"], - redis_db=self.redis_params["db"], - redis_password=self.redis_params["password"], + redis_params=self.redis_params, ) state_update = self.routes[queue_name].run( diff --git a/motion/state/state.py b/motion/state/state.py index ccdc52b1..7d4e8bd7 100644 --- a/motion/state/state.py +++ b/motion/state/state.py @@ -1,7 +1,7 @@ """ This file contains the state class. """ -from typing import Any, Iterator, List, Optional, Tuple +from typing import Any, Dict, Iterator, List, Optional, Tuple from motionstate import StateAccessor # type: ignore @@ -41,10 +41,7 @@ def __init__( self, component_name: str, instance_id: str, - redis_host: str, - redis_port: int, - redis_db: int = 0, - redis_password: Optional[str] = None, + redis_params: Dict[str, Any], *args: Any, **kwargs: Any, ) -> None: @@ -54,10 +51,11 @@ def __init__( component_name, instance_id, 1000 * 60 * 2, # 2 minutes lock duration TODO: make this configurable - redis_host, - redis_port, - redis_db, - redis_password, + redis_params["host"], + redis_params["port"], + redis_params["db"], + redis_params["password"], + redis_params.get("ssl", False), ) super().__init__(*args, **kwargs) diff --git a/motion/utils.py b/motion/utils.py index df464cce..d4aeca02 100644 --- a/motion/utils.py +++ b/motion/utils.py @@ -218,10 +218,7 @@ def inspect_state(instance_name: str) -> Dict[str, Any]: state = State( instance_name.split("__")[0], instance_name.split("__")[1], - rp.host, - rp.port, - rp.db, - rp.password, + redis_params=rp.dict(), ) # Iterate through all items all_items = {k: v for k, v in state.items()} diff --git a/motionstate/src/lib.rs b/motionstate/src/lib.rs index 37b5620c..f8b7e7bd 100644 --- a/motionstate/src/lib.rs +++ b/motionstate/src/lib.rs @@ -31,7 +31,7 @@ pub struct StateAccessor { lock_duration: usize, version: u64, client: redis::Client, - cache: HashMap>>, + cache: HashMap, // Stores deserialized values lock_manager: RedLock, max_lock_attempts: u32, } @@ -47,14 +47,18 @@ impl StateAccessor { redis_port: u16, redis_db: i64, redis_password: Option<&str>, + redis_ssl: Option, ) -> PyResult { - // Constructing the Redis URL + let use_ssl: bool = redis_ssl.unwrap_or(false); + let protocol: &str = if use_ssl { "rediss" } else { "redis" }; + + // Constructing the Redis URL with SSL consideration let redis_url = match redis_password { Some(password) => format!( - "redis://:{}@{}:{}/{}", - password, redis_host, redis_port, redis_db + "{}://:{}@{}:{}/{}", + protocol, password, redis_host, redis_port, redis_db ), - None => format!("redis://{}:{}/{}", redis_host, redis_port, redis_db), + None => format!("{}://{}:{}/{}", protocol, redis_host, redis_port, redis_db), }; let client = redis::Client::open(redis_url.clone()).map_err(|err| { @@ -139,7 +143,7 @@ impl StateAccessor { // Critical section // Insert the key and value into the cache - self.cache.insert(keyname.clone(), serialized_data.clone()); + self.cache.insert(keyname.clone(), value.into_py(py)); // Increment the version and write it to Redis self.version += 1; @@ -253,8 +257,19 @@ impl StateAccessor { // Critical section for (keyname, serialized_data, ttl) in serialized_items.iter() { + let unserialized_value = items + .get_item(keyname.replace( + &format!( + "MOTION_STATE:{}__{}/", + self.component_name, self.instance_id + ), + "", + )) + .unwrap(); + // Insert the key and value into the cache - self.cache.insert(keyname.clone(), serialized_data.clone()); + self.cache + .insert(keyname.clone(), unserialized_value.into_py(py)); // If ttl is not None, set the TTL if let Some(ttl) = ttl { @@ -318,9 +333,9 @@ impl StateAccessor { self.component_name, self.instance_id, key ); - // If the key is in the cache, return it + // Return the cached object if it exists if let Some(value) = self.cache.get(&keyname) { - return deserialize_value(py, &*value); + return Ok(value.clone_ref(py)); } // Otherwise, fetch it from Redis @@ -329,12 +344,14 @@ impl StateAccessor { match result_data { Ok(Some(data)) => { - let data_arc = Arc::new(data); - - // Insert the key and value into the cache - self.cache.insert(keyname.clone(), data_arc.clone()); // Deserialize the value - deserialize_value(py, &*data_arc) + let deserialized_value = deserialize_value(py, &data)?; + + // Insert the deserialized value into the cache + self.cache + .insert(keyname.clone(), deserialized_value.clone_ref(py)); + + Ok(deserialized_value) } Ok(None) => Err(PyErr::new::("Key not found")), Err(err) => Err(PyErr::new::(format!( diff --git a/tests/state/test_rust_vs_cloudpickle.py b/tests/state/test_rust_vs_cloudpickle.py new file mode 100644 index 00000000..ea618cdb --- /dev/null +++ b/tests/state/test_rust_vs_cloudpickle.py @@ -0,0 +1,51 @@ +from motion import Component + +import pytest +import time +import random +import numpy as np +import copy + +FragmentedState = Component("FragmentedState") +UnifiedState = Component("UnifiedState") + +NUM_KEYS = 1000 +VECTOR_LEN = 10000 +D = { + str(i): np.array([random.random() for _ in range(VECTOR_LEN)]) + for i in range(NUM_KEYS) +} +print("Done generating keys") + + +@FragmentedState.init_state +def setupf(): + # Make a bunch of keys and values + return D + + +@UnifiedState.init_state +def setupu(): + d = copy.deepcopy(D) + return {"state": d} + + +def test_key_level_serialization_faster(): + fs = FragmentedState() + print("Done initializing FragmentedState") + us = UnifiedState() + print("Done initializing UnifiedState") + + # Time how long it takes to read a key + start = time.time() + fs.read_state("0") + end = time.time() + key_level_time = end - start + + # Time how long it takes to serialize the same dict and read it + start = time.time() + us.read_state("state")["0"] + end = time.time() + unified_level_time = end - start + + assert key_level_time < unified_level_time diff --git a/unnecessary.py b/unnecessary.py new file mode 100644 index 00000000..76a4dc47 --- /dev/null +++ b/unnecessary.py @@ -0,0 +1,334 @@ +from typing import Any, Awaitable, Dict, Optional + +import httpx +import requests + + +class ComponentInstanceClient: + def __init__( + self, + component_name: str, + instance_id: str, + uri: str, + access_token: str, + **kwargs: Any, + ): + """Creates a new instance of a Motion component. + + Args: + component_name (str): + Name of the component we are creating an instance of. + instance_id (str): + ID of the instance we are creating. + """ + self._component_name = component_name + + # Create instance name + self._instance_name = f"{self._component_name}__{instance_id}" + + self.uri = uri + self.access_token = access_token + + self.kwargs = kwargs + + @property + def instance_name(self) -> str: + """Component name with a random phrase to represent + the name of this instance. + In the form of componentname__randomphrase. + """ + return self._instance_name + + @property + def instance_id(self) -> str: + """Latter part of the instance name, which is a random phrase + or a user-defined ID.""" + return self._instance_name.split("__")[-1] + + def write_state(self, state_update: Dict[str, Any], latest: bool = False) -> None: + """Writes the state update to the component instance's state. + If a update op is currently running, the state update will be + applied after the update op is finished. Warning: this could + take a while if your update ops take a long time! + + Usage: + ```python + from motion import Component + + C = Component("MyComponent") + + @C.init_state + def setUp(): + return {"value": 0} + + # Define serve and update operations + ... + + if __name__ == "__main__": + c_instance = C() + c_instance.read_state("value") # Returns 0 + c_instance.write_state({"value": 1, "value2": 2}) + c_instance.read_state("value") # Returns 1 + c_instance.read_state("value2") # Returns 2 + ``` + + Args: + state_update (Dict[str, Any]): Dictionary of key-value pairs + to update the state with. + latest (bool, optional): Whether or not to apply the update + to the latest version of the state. + If true, Motion will redownload the latest version + of the state and apply the update to that version. You + only need to set this to true if you are updating an + instance you connected to a while ago and might be + outdated. Defaults to False. + """ + # Ask server to update state + response = requests.post( + f"{self.uri}/update_state", + json={ + "instance_id": self.instance_id, + "state_update": state_update, + "kwargs": {"latest": latest}, + }, + headers={"Authorization": f"Bearer {self.access_token}"}, + ) + if response.status_code != 200: + raise RuntimeError( + f"Failed to update state for instance {self.instance_id}: {response.text}" + ) + + def read_state(self, key: str, default_value: Optional[Any] = None) -> Any: + """Gets the current value for the key in the component instance's state. + + Usage: + ```python + from motion import Component + + C = Component("MyComponent") + + @C.init_state + def setUp(): + return {"value": 0} + + # Define serve and update operations + ... + + if __name__ == "__main__": + c_instance = C() + c_instance.read_state("value") # Returns 0 + c_instance.run(...) + c_instance.read_state("value") # This will return the current value + # of "value" in the state + ``` + + Args: + key (str): Key in the state to get the value for. + default_value (Optional[Any], optional): Default value to return + if the key is not found. Defaults to None. + + Returns: + Any: Current value for the key, or default_value if the key + is not found. + """ + # Ask server to read state + response = requests.get( + f"{self.uri}/read_state", + params={"instance_id": self.instance_id, "key": key}, + headers={"Authorization": f"Bearer {self.access_token}"}, + ) + if response.status_code != 200: + raise RuntimeError( + f"Failed to read state for instance {self.instance_id}: {response.text}" + ) + + # Get response + result = response.json()["value"] + if not result: + return default_value + + return result + + def run( + self, + # *, + dataflow_key: str, + props: Dict[str, Any] = {}, + ignore_cache: bool = False, + force_refresh: bool = False, + flush_update: bool = False, + ) -> Any: + """Runs the dataflow (serve and update ops) for the keyword argument + passed in. If the key is not found to have any ops, an error + is raised. Only one dataflow key should be passed in. + + Example Usage: + ```python + from motion import Component + + C = Component("MyComponent") + + @C.init_state + def setUp(): + return {"value": 0} + + @C.serve("add") + def add(state, value): + return state["value"] + value + + @C.update("add") + def add(state, value): + return {"value": state["value"] + value} + + if __name__ == "__main__": + c = C() # Create instance of C + c.run("add", props={"value": 1}, flush_update=True) # (1)! + c.run("add", props={"value": 1}) # Returns 1 + c.run("add", props={"value": 2}, flush_update=True) # (2)! + + c.run("add", props={"value": 3}) + time.sleep(3) # Wait for the previous update op to finish + + c.run("add", props={"value": 3}, force_refresh=True) # (3)! + + # 1. Waits for the update op to finish, then updates the state + # 2. Returns 2, result state["value"] = 4 + # 3. Force refreshes the state before running the dataflow, and + # reruns the serve op even though the result might be cached. + ``` + + + Args: + dataflow_key (str): Key of the dataflow to run. + props (Dict[str, Any]): Keyword arguments to pass into the + dataflow ops, in addition to the state. + ignore_cache (bool, optional): + If True, ignores the cache and runs the serve op. Does not + force refresh the state. Defaults to False. + force_refresh (bool, optional): Read the latest value of the + state before running an serve call, otherwise a stale + version of the state or a cached result may be used. + Defaults to False. + flush_update (bool, optional): + If True, waits for the update op to finish executing before + returning. If the update queue hasn't reached batch_size + yet, the update op runs anyways. Force refreshes the + state after the update op completes. Defaults to False. + + Raises: + ValueError: If more than one dataflow key-value pair is passed. + RuntimeError: + If flush_update is called and the component instance update + processes are disabled. + + Returns: + Any: Result of the serve call. Might take a long time + to run if `flush_update = True` and the update operation is + computationally expensive. + """ + + # Ask server to run dataflow + response = requests.post( + f"{self.uri}/{self.component_name}", + json={ + "component_name": self.component_name, + "instance_id": self.instance_id, + "dataflow_key": dataflow_key, + "is_async": False, + "props": props, + "kwargs": { + "ignore_cache": ignore_cache, + "force_refresh": force_refresh, + "flush_update": flush_update, + }, + }, + headers={"Authorization": f"Bearer {self.access_token}"}, + ) + if response.status_code != 200: + raise RuntimeError( + f"Failed to run dataflow for instance {self.instance_id}: {response.text}" + ) + + # Get response + result = response.json()["value"] + return result + + async def arun( + self, + # *, + dataflow_key: str, + props: Dict[str, Any] = {}, + ignore_cache: bool = False, + force_refresh: bool = False, + flush_update: bool = False, + ) -> Awaitable[Any]: + """Async version of run. Runs the dataflow (serve and update ops) for + the specified key. You should use arun if either the serve or update op + is an async function. + + Example Usage: + ```python + from motion import Component + import asyncio + + C = Component("MyComponent") + + @C.serve("sleep") + async def sleep(state, value): + await asyncio.sleep(value) + return "Slept!" + + async def main(): + c = C() + await c.arun("sleep", props={"value": 1}) + + if __name__ == "__main__": + asyncio.run(main()) + ``` + + Args: + dataflow_key (str): Key of the dataflow to run. + props (Dict[str, Any]): Keyword arguments to pass into the + dataflow ops, in addition to the state. + ignore_cache (bool, optional): + If True, ignores the cache and runs the serve op. Does not + force refresh the state. Defaults to False. + force_refresh (bool, optional): Read the latest value of the + state before running an serve call, otherwise a stale + version of the state or a cached result may be used. + Defaults to False. + flush_update (bool, optional): + If True, waits for the update op to finish executing before + returning. If the update queue hasn't reached batch_size + yet, the update op runs anyways. Force refreshes the + state after the update op completes. Defaults to False. + + Raises: + ValueError: If more than one dataflow key-value pair is passed. + If flush_update is called and the component instance update + processes are disabled. + + Returns: + Awaitable[Any]: Awaitable Result of the serve call. + """ + + # Ask server to run dataflow asynchronously + async with httpx.AsyncClient() as client: + response = await client.post( + f"{self.uri}/{self.component_name}", + json={ + "component_name": self.component_name, + "instance_id": self.instance_id, + "dataflow_key": dataflow_key, + "is_async": True, + "props": props, + "kwargs": { + "ignore_cache": ignore_cache, + "force_refresh": force_refresh, + "flush_update": flush_update, + }, + }, + headers={"Authorization": f"Bearer {self.access_token}"}, + ) + response.raise_for_status() + return response.json()["value"] From d032240bc1eb21aac6f9f2a0fc539bd30ffb40a1 Mon Sep 17 00:00:00 2001 From: Shreya Shankar Date: Fri, 24 Nov 2023 17:14:43 -0600 Subject: [PATCH 14/37] Modify pyproject.toml --- pyproject.toml | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 60a2d2d4..767ab95a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -71,8 +71,13 @@ show_error_codes = true omit = [".*", "*/site-packages/*"] [build-system] -requires = ["poetry-core"] -build-backend = "poetry.core.masonry.api" +requires = ["poetry-core", "maturin>=0.12"] +build-backend = "maturin" + +[tool.maturin] +name = "motionstate" +sdist = true +bindings = "pyo3" [tool.coverage.report] exclude_lines = [ From c0d8a7aaef4b1efb98b9191e5f4e975b129ca528 Mon Sep 17 00:00:00 2001 From: Shreya Shankar Date: Fri, 24 Nov 2023 17:24:16 -0600 Subject: [PATCH 15/37] Try to run CI --- .github/workflows/ci.yml | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e37164a9..e918f6dd 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -35,9 +35,22 @@ jobs: - name: Install Poetry uses: snok/install-poetry@v1 + - name: Install Rust toolchain + uses: actions-rs/toolchain@v1 + with: + toolchain: stable + profile: minimal + override: true + - name: Install dependencies run: poetry install + - name: Build and install Rust library + run: | + cd motionstate + maturin develop --release + cd .. + - name: Run pytest run: make tests From ad73b0ad400b78d5fdfb86eac87edbe65d676e98 Mon Sep 17 00:00:00 2001 From: Shreya Shankar Date: Fri, 24 Nov 2023 17:27:06 -0600 Subject: [PATCH 16/37] Try to run CI --- .github/workflows/ci.yml | 3 +++ pyproject.toml | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e918f6dd..1780ca0b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -45,6 +45,9 @@ jobs: - name: Install dependencies run: poetry install + - name: Install Maturin + run: pip install maturin + - name: Build and install Rust library run: | cd motionstate diff --git a/pyproject.toml b/pyproject.toml index 767ab95a..e361b4cc 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -71,7 +71,7 @@ show_error_codes = true omit = [".*", "*/site-packages/*"] [build-system] -requires = ["poetry-core", "maturin>=0.12"] +requires = ["poetry-core", "maturin"] build-backend = "maturin" [tool.maturin] From 9299ca015eeef0243ed6a5dc144298951161852f Mon Sep 17 00:00:00 2001 From: Shreya Shankar Date: Fri, 24 Nov 2023 17:29:57 -0600 Subject: [PATCH 17/37] Try to run CI --- .github/workflows/ci.yml | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1780ca0b..914487b1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -45,13 +45,10 @@ jobs: - name: Install dependencies run: poetry install - - name: Install Maturin - run: pip install maturin - - name: Build and install Rust library run: | cd motionstate - maturin develop --release + poetry run maturin develop --release cd .. - name: Run pytest From 751c2fb57ae034ff32a1d0e69abcc0dacca38332 Mon Sep 17 00:00:00 2001 From: Shreya Shankar Date: Fri, 24 Nov 2023 18:06:21 -0600 Subject: [PATCH 18/37] Restructure to match maturin documentation for mixed libraries --- .github/workflows/ci.yml | 6 +- .gitignore | 5 +- Cargo.lock | 487 ++++++++++++++++++++++++ motionstate/Cargo.toml => Cargo.toml | 1 + motion/__init__.py | 2 + motion/motion.pyi | 2 + motion/state/__init__.py | 3 +- motion/state/state.py | 2 +- pyproject.toml | 7 +- {motionstate/src => src}/lib.rs | 2 +- {motionstate/src => src}/state_value.rs | 0 {motionstate/src => src}/temp_value.rs | 0 tests/state/test_temp_value.py | 2 +- 13 files changed, 502 insertions(+), 17 deletions(-) create mode 100644 Cargo.lock rename motionstate/Cargo.toml => Cargo.toml (95%) create mode 100644 motion/motion.pyi rename {motionstate/src => src}/lib.rs (99%) rename {motionstate/src => src}/state_value.rs (100%) rename {motionstate/src => src}/temp_value.rs (100%) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 914487b1..d7b0b058 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -45,11 +45,9 @@ jobs: - name: Install dependencies run: poetry install - - name: Build and install Rust library + - name: Build and install run: | - cd motionstate - poetry run maturin develop --release - cd .. + poetry run maturin develop - name: Run pytest run: make tests diff --git a/.gitignore b/.gitignore index 142fe0bf..986599ed 100644 --- a/.gitignore +++ b/.gitignore @@ -20,8 +20,9 @@ projects motionstate/debug* motionstate/target* motionstate/Cargo.lock +*.so *rustc* -motionstate*.so *motionenv* *.whl -*unnecessary* \ No newline at end of file +*unnecessary* +target* \ No newline at end of file diff --git a/Cargo.lock b/Cargo.lock new file mode 100644 index 00000000..26918383 --- /dev/null +++ b/Cargo.lock @@ -0,0 +1,487 @@ +# This file is automatically @generated by Cargo. +# It is not intended for manual editing. +version = 3 + +[[package]] +name = "autocfg" +version = "1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d468802bab17cbc0cc575e9b053f41e72aa36bfa6b7f55e3529ffa43161b97fa" + +[[package]] +name = "bincode" +version = "1.3.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b1f45e9417d87227c7a56d22e471c6206462cba514c7590c09aff4cf6d1ddcad" +dependencies = [ + "serde", +] + +[[package]] +name = "bitflags" +version = "1.3.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bef38d45163c2f1dde094a7dfd33ccf595c92905c8f8f4fdc18d06fb1037718a" + +[[package]] +name = "bytes" +version = "1.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "89b2fd2a0dcf38d7971e2194b6b6eebab45ae01067456a7fd93d5547a61b70be" + +[[package]] +name = "cfg-if" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "baf1de4339761588bc0619e3cbc0120ee582ebb74b53b4efbf79117bd2da40fd" + +[[package]] +name = "combine" +version = "4.6.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "35ed6e9d84f0b51a7f52daf1c7d71dd136fd7a3f41a8462b8cdb8c78d920fad4" +dependencies = [ + "bytes", + "memchr", +] + +[[package]] +name = "form_urlencoded" +version = "1.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a62bc1cf6f830c2ec14a513a9fb124d0a213a629668a4186f329db21fe045652" +dependencies = [ + "percent-encoding", +] + +[[package]] +name = "getrandom" +version = "0.2.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "be4136b2a15dd319360be1c07d9933517ccf0be8f16bf62a3bee4f0d618df427" +dependencies = [ + "cfg-if", + "libc", + "wasi", +] + +[[package]] +name = "idna" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7d20d6b07bfbc108882d88ed8e37d39636dcc260e15e30c45e6ba089610b917c" +dependencies = [ + "unicode-bidi", + "unicode-normalization", +] + +[[package]] +name = "indoc" +version = "1.0.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bfa799dd5ed20a7e349f3b4639aa80d74549c81716d9ec4f994c9b5815598306" + +[[package]] +name = "instant" +version = "0.1.12" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7a5bbe824c507c5da5956355e86a746d82e0e1464f65d862cc5e71da70e94b2c" +dependencies = [ + "cfg-if", +] + +[[package]] +name = "itoa" +version = "1.0.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "af150ab688ff2122fcef229be89cb50dd66af9e01a4ff320cc137eecc9bacc38" + +[[package]] +name = "libc" +version = "0.2.147" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b4668fb0ea861c1df094127ac5f1da3409a82116a4ba74fca2e58ef927159bb3" + +[[package]] +name = "lock_api" +version = "0.4.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c1cc9717a20b1bb222f333e6a92fd32f7d8a18ddc5a3191a11af45dcbf4dcd16" +dependencies = [ + "autocfg", + "scopeguard", +] + +[[package]] +name = "memchr" +version = "2.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2dffe52ecf27772e601905b7522cb4ef790d2cc203488bbd0e2fe85fcb74566d" + +[[package]] +name = "memoffset" +version = "0.9.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5a634b1c61a95585bd15607c6ab0c4e5b226e695ff2800ba0cdccddf208c406c" +dependencies = [ + "autocfg", +] + +[[package]] +name = "motionstate" +version = "0.1.0" +dependencies = [ + "bincode", + "pyo3", + "redis", + "redlock", + "serde", +] + +[[package]] +name = "once_cell" +version = "1.18.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dd8b5dd2ae5ed71462c540258bedcb51965123ad7e7ccf4b9a8cafaa4a63576d" + +[[package]] +name = "parking_lot" +version = "0.11.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7d17b78036a60663b797adeaee46f5c9dfebb86948d1255007a1d6be0271ff99" +dependencies = [ + "instant", + "lock_api", + "parking_lot_core", +] + +[[package]] +name = "parking_lot_core" +version = "0.8.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "60a2cfe6f0ad2bfc16aefa463b497d5c7a5ecd44a23efa72aa342d90177356dc" +dependencies = [ + "cfg-if", + "instant", + "libc", + "redox_syscall", + "smallvec", + "winapi", +] + +[[package]] +name = "percent-encoding" +version = "2.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9b2a4787296e9989611394c33f193f676704af1686e70b8f8033ab5ba9a35a94" + +[[package]] +name = "ppv-lite86" +version = "0.2.17" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5b40af805b3121feab8a3c29f04d8ad262fa8e0561883e7653e024ae4479e6de" + +[[package]] +name = "proc-macro2" +version = "1.0.69" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "134c189feb4956b20f6f547d2cf727d4c0fe06722b20a0eec87ed445a97f92da" +dependencies = [ + "unicode-ident", +] + +[[package]] +name = "pyo3" +version = "0.19.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e681a6cfdc4adcc93b4d3cf993749a4552018ee0a9b65fc0ccfad74352c72a38" +dependencies = [ + "cfg-if", + "indoc", + "libc", + "memoffset", + "parking_lot", + "pyo3-build-config", + "pyo3-ffi", + "pyo3-macros", + "unindent", +] + +[[package]] +name = "pyo3-build-config" +version = "0.19.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "076c73d0bc438f7a4ef6fdd0c3bb4732149136abd952b110ac93e4edb13a6ba5" +dependencies = [ + "once_cell", + "target-lexicon", +] + +[[package]] +name = "pyo3-ffi" +version = "0.19.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e53cee42e77ebe256066ba8aa77eff722b3bb91f3419177cf4cd0f304d3284d9" +dependencies = [ + "libc", + "pyo3-build-config", +] + +[[package]] +name = "pyo3-macros" +version = "0.19.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dfeb4c99597e136528c6dd7d5e3de5434d1ceaf487436a3f03b2d56b6fc9efd1" +dependencies = [ + "proc-macro2", + "pyo3-macros-backend", + "quote", + "syn 1.0.109", +] + +[[package]] +name = "pyo3-macros-backend" +version = "0.19.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "947dc12175c254889edc0c02e399476c2f652b4b9ebd123aa655c224de259536" +dependencies = [ + "proc-macro2", + "quote", + "syn 1.0.109", +] + +[[package]] +name = "quote" +version = "1.0.33" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5267fca4496028628a95160fc423a33e8b2e6af8a5302579e322e4b520293cae" +dependencies = [ + "proc-macro2", +] + +[[package]] +name = "rand" +version = "0.8.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "34af8d1a0e25924bc5b7c43c079c942339d8f0a8b57c39049bef581b46327404" +dependencies = [ + "libc", + "rand_chacha", + "rand_core", +] + +[[package]] +name = "rand_chacha" +version = "0.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e6c10a63a0fa32252be49d21e7709d4d4baf8d231c2dbce1eaa8141b9b127d88" +dependencies = [ + "ppv-lite86", + "rand_core", +] + +[[package]] +name = "rand_core" +version = "0.6.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ec0be4795e2f6a28069bec0b5ff3e2ac9bafc99e6a9a7dc3547996c5c816922c" +dependencies = [ + "getrandom", +] + +[[package]] +name = "redis" +version = "0.23.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ffd6543a7bc6428396845f6854ccf3d1ae8823816592e2cbe74f20f50f209d02" +dependencies = [ + "combine", + "itoa", + "percent-encoding", + "ryu", + "sha1_smol", + "socket2", + "url", +] + +[[package]] +name = "redlock" +version = "2.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ce310b27b5923ad1cd21b0e007fba6b6a9926f773099d867fb0477cc188b8aa2" +dependencies = [ + "rand", + "redis", +] + +[[package]] +name = "redox_syscall" +version = "0.2.16" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fb5a58c1855b4b6819d59012155603f0b22ad30cad752600aadfcb695265519a" +dependencies = [ + "bitflags", +] + +[[package]] +name = "ryu" +version = "1.0.15" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1ad4cc8da4ef723ed60bced201181d83791ad433213d8c24efffda1eec85d741" + +[[package]] +name = "scopeguard" +version = "1.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "94143f37725109f92c262ed2cf5e59bce7498c01bcc1502d7b9afe439a4e9f49" + +[[package]] +name = "serde" +version = "1.0.193" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "25dd9975e68d0cb5aa1120c288333fc98731bd1dd12f561e468ea4728c042b89" +dependencies = [ + "serde_derive", +] + +[[package]] +name = "serde_derive" +version = "1.0.193" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "43576ca501357b9b071ac53cdc7da8ef0cbd9493d8df094cd821777ea6e894d3" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.39", +] + +[[package]] +name = "sha1_smol" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ae1a47186c03a32177042e55dbc5fd5aee900b8e0069a8d70fba96a9375cd012" + +[[package]] +name = "smallvec" +version = "1.11.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "62bb4feee49fdd9f707ef802e22365a35de4b7b299de4763d44bfea899442ff9" + +[[package]] +name = "socket2" +version = "0.4.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "64a4a911eed85daf18834cfaa86a79b7d266ff93ff5ba14005426219480ed662" +dependencies = [ + "libc", + "winapi", +] + +[[package]] +name = "syn" +version = "1.0.109" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "72b64191b275b66ffe2469e8af2c1cfe3bafa67b529ead792a6d0160888b4237" +dependencies = [ + "proc-macro2", + "quote", + "unicode-ident", +] + +[[package]] +name = "syn" +version = "2.0.39" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "23e78b90f2fcf45d3e842032ce32e3f2d1545ba6636271dcbf24fa306d87be7a" +dependencies = [ + "proc-macro2", + "quote", + "unicode-ident", +] + +[[package]] +name = "target-lexicon" +version = "0.12.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9d0e916b1148c8e263850e1ebcbd046f333e0683c724876bb0da63ea4373dc8a" + +[[package]] +name = "tinyvec" +version = "1.6.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "87cc5ceb3875bb20c2890005a4e226a4651264a5c75edb2421b52861a0a0cb50" +dependencies = [ + "tinyvec_macros", +] + +[[package]] +name = "tinyvec_macros" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1f3ccbac311fea05f86f61904b462b55fb3df8837a366dfc601a0161d0532f20" + +[[package]] +name = "unicode-bidi" +version = "0.3.13" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "92888ba5573ff080736b3648696b70cafad7d250551175acbaa4e0385b3e1460" + +[[package]] +name = "unicode-ident" +version = "1.0.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "301abaae475aa91687eb82514b328ab47a211a533026cb25fc3e519b86adfc3c" + +[[package]] +name = "unicode-normalization" +version = "0.1.22" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5c5713f0fc4b5db668a2ac63cdb7bb4469d8c9fed047b1d0292cc7b0ce2ba921" +dependencies = [ + "tinyvec", +] + +[[package]] +name = "unindent" +version = "0.1.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e1766d682d402817b5ac4490b3c3002d91dfa0d22812f341609f97b08757359c" + +[[package]] +name = "url" +version = "2.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "50bff7831e19200a85b17131d085c25d7811bc4e186efdaf54bbd132994a88cb" +dependencies = [ + "form_urlencoded", + "idna", + "percent-encoding", +] + +[[package]] +name = "wasi" +version = "0.11.0+wasi-snapshot-preview1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9c8d87e72b64a3b4db28d11ce29237c246188f4f51057d65a7eab63b7987e423" + +[[package]] +name = "winapi" +version = "0.3.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5c839a674fcd7a98952e593242ea400abe93992746761e38641405d28b00f419" +dependencies = [ + "winapi-i686-pc-windows-gnu", + "winapi-x86_64-pc-windows-gnu", +] + +[[package]] +name = "winapi-i686-pc-windows-gnu" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ac3b87c63620426dd9b991e5ce0329eff545bccbbb34f3be09ff6fb6ab51b7b6" + +[[package]] +name = "winapi-x86_64-pc-windows-gnu" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "712e227841d057c1ee1cd2fb22fa7e5a5461ae8e48fa2ca79ec42cfc1931183f" diff --git a/motionstate/Cargo.toml b/Cargo.toml similarity index 95% rename from motionstate/Cargo.toml rename to Cargo.toml index dc90ff81..59961aef 100644 --- a/motionstate/Cargo.toml +++ b/Cargo.toml @@ -6,6 +6,7 @@ edition = "2021" # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html [lib] +name = "motion" crate-type = ["cdylib"] [dependencies] diff --git a/motion/__init__.py b/motion/__init__.py index cd3ed39a..9289593c 100644 --- a/motion/__init__.py +++ b/motion/__init__.py @@ -11,6 +11,7 @@ from motion.dicts import MDataFrame from motion.copy_utils import copy_db from motion.server.application import Application +from .motion import TempValue __all__ = [ "Component", @@ -24,4 +25,5 @@ "copy_db", "RedisParams", "Application", + "TempValue", ] diff --git a/motion/motion.pyi b/motion/motion.pyi new file mode 100644 index 00000000..9644c671 --- /dev/null +++ b/motion/motion.pyi @@ -0,0 +1,2 @@ +class StateAccessor: ... +class TempValue: ... diff --git a/motion/state/__init__.py b/motion/state/__init__.py index 12769269..163c1e51 100644 --- a/motion/state/__init__.py +++ b/motion/state/__init__.py @@ -1,4 +1,3 @@ from motion.state.state import State -from motionstate import TempValue # type: ignore -__all__ = ["State", "TempValue"] +__all__ = ["State"] diff --git a/motion/state/state.py b/motion/state/state.py index 7d4e8bd7..77ddccea 100644 --- a/motion/state/state.py +++ b/motion/state/state.py @@ -3,7 +3,7 @@ """ from typing import Any, Dict, Iterator, List, Optional, Tuple -from motionstate import StateAccessor # type: ignore +from ..motion import StateAccessor # type: ignore STATE_ERROR_MSG = "Cannot edit state directly. Use component update operations instead." diff --git a/pyproject.toml b/pyproject.toml index e361b4cc..28fb4aa6 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -71,14 +71,9 @@ show_error_codes = true omit = [".*", "*/site-packages/*"] [build-system] -requires = ["poetry-core", "maturin"] +requires = ["maturin>=0.14,<0.15"] build-backend = "maturin" -[tool.maturin] -name = "motionstate" -sdist = true -bindings = "pyo3" - [tool.coverage.report] exclude_lines = [ "pragma: no cover", diff --git a/motionstate/src/lib.rs b/src/lib.rs similarity index 99% rename from motionstate/src/lib.rs rename to src/lib.rs index f8b7e7bd..4bf66114 100644 --- a/motionstate/src/lib.rs +++ b/src/lib.rs @@ -527,7 +527,7 @@ fn deserialize_value(py: Python, value: &[u8]) -> PyResult { } #[pymodule] -fn motionstate(_py: Python, m: &PyModule) -> PyResult<()> { +fn motion(_py: Python, m: &PyModule) -> PyResult<()> { m.add_class::()?; // m.add_class::()?; m.add_class::()?; diff --git a/motionstate/src/state_value.rs b/src/state_value.rs similarity index 100% rename from motionstate/src/state_value.rs rename to src/state_value.rs diff --git a/motionstate/src/temp_value.rs b/src/temp_value.rs similarity index 100% rename from motionstate/src/temp_value.rs rename to src/temp_value.rs diff --git a/tests/state/test_temp_value.py b/tests/state/test_temp_value.py index 25bce547..2cbbc656 100644 --- a/tests/state/test_temp_value.py +++ b/tests/state/test_temp_value.py @@ -1,5 +1,5 @@ from motion import Component -from motion.state import TempValue +from motion import TempValue import pytest import time From 12efa49c84becf1d2ec3bd324420636bca25b1f0 Mon Sep 17 00:00:00 2001 From: Shreya Shankar Date: Fri, 24 Nov 2023 18:20:25 -0600 Subject: [PATCH 19/37] Restructure to match maturin documentation for mixed libraries --- Makefile | 12 +---------- docs/api/application.md | 3 +-- docs/api/props-and-state.md | 4 +--- motion/motion.pyi | 43 +++++++++++++++++++++++++++++++++++-- 4 files changed, 44 insertions(+), 18 deletions(-) diff --git a/Makefile b/Makefile index 4ee0370a..d4bcf3b0 100644 --- a/Makefile +++ b/Makefile @@ -20,14 +20,4 @@ docs: poetry run mkdocs serve build: - cd motionstate && \ - echo "Building motionstate" && \ - maturin develop && \ - maturin build --release --interpreter python3.9 python3.10 python3.11 && \ - python -m venv ../.motionenv && \ - source ../.motionenv/bin/activate && pip install target/wheels/motionstate*.whl && deactivate && \ - echo "Copying .so file to motion" && \ - cp $$(find ../.motionenv -name "motionstate*.so") ../motion/ && \ - echo "Cleanup virtual environment..." && \ - rm -rf ../.motionenv && \ - echo "Done" + poetry run maturin develop diff --git a/docs/api/application.md b/docs/api/application.md index f15d0f8c..daa9e2d2 100644 --- a/docs/api/application.md +++ b/docs/api/application.md @@ -1,5 +1,4 @@ -::: motion.server.Application - handler: python +::: motion.Application options: members: - __init__ diff --git a/docs/api/props-and-state.md b/docs/api/props-and-state.md index 4387662d..483c2281 100644 --- a/docs/api/props-and-state.md +++ b/docs/api/props-and-state.md @@ -9,11 +9,9 @@ show_source: false show_signature_annotations: true -::: motion.dicts.State +::: motion.state.State handler: python options: - members: - - instance_id show_root_full_path: false show_root_toc_entry: false show_root_heading: true diff --git a/motion/motion.pyi b/motion/motion.pyi index 9644c671..c7f99a69 100644 --- a/motion/motion.pyi +++ b/motion/motion.pyi @@ -1,2 +1,41 @@ -class StateAccessor: ... -class TempValue: ... +from typing import Any, Dict, List, Optional, Tuple +from pyo3 import PyObject + +class StateAccessor: + component_name: str + instance_id: str + lock_duration: int + client: Any # Figure out redis type + cache: Dict[str, PyObject] + lock_manager: Any # Figure out redis type + max_lock_attempts: int + + def __init__( + self, + component_name: str, + instance_id: str, + lock_duration: int, + redis_host: str, + redis_port: int, + redis_db: int, + redis_password: Optional[str] = None, + redis_ssl: Optional[bool] = None, + ) -> None: ... + @property + def version(self) -> int: ... + def set(self, key: str, value: Any) -> None: ... + def bulk_set(self, items: Dict[str, Any], from_migration: bool = False) -> None: ... + def get(self, key: str) -> Any: ... + def items(self) -> List[Tuple[str, Any]]: ... + def keys(self) -> List[str]: ... + def values(self) -> List[Any]: ... + def clear_cache(self) -> None: ... + +class TempValue: + def __init__(self, value: PyObject, ttl: int) -> None: ... + @property + def value(self) -> PyObject: ... + @property + def ttl(self) -> int: ... + @ttl.setter + def ttl(self, new_ttl: int) -> None: ... From c21eb9747b017ced4d986de897e172ad6de01877 Mon Sep 17 00:00:00 2001 From: Shreya Shankar Date: Fri, 24 Nov 2023 18:22:35 -0600 Subject: [PATCH 20/37] Change ci file to use make commands --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d7b0b058..66fb3b03 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -47,7 +47,7 @@ jobs: - name: Build and install run: | - poetry run maturin develop + make build - name: Run pytest run: make tests From 632a2935f45de1c920cc9baf8c58662ef42816ba Mon Sep 17 00:00:00 2001 From: Shreya Shankar Date: Fri, 24 Nov 2023 18:54:02 -0600 Subject: [PATCH 21/37] Check that version is bumped up automatically --- .github/workflows/ci.yml | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 66fb3b03..29194c19 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1,4 +1,4 @@ -name: motion +name: CI on: workflow_dispatch: @@ -19,6 +19,30 @@ jobs: - name: Checkout code uses: actions/checkout@v2 + - name: Fetch main branch + run: git fetch origin main + + - name: Get current version from main branch + id: main_version + run: | + MAIN_VERSION=$(git show origin/main:pyproject.toml | grep -Po 'version = "\K[^"]+') + echo "Main branch version: $MAIN_VERSION" + echo "MAIN_VERSION=$MAIN_VERSION" >> $GITHUB_ENV + + - name: Get version from current branch + id: current_version + run: | + CURRENT_VERSION=$(grep -Po 'version = "\K[^"]+' pyproject.toml) + echo "Current branch version: $CURRENT_VERSION" + echo "CURRENT_VERSION=$CURRENT_VERSION" >> $GITHUB_ENV + + - name: Check if version is bumped + run: | + if [[ "$CURRENT_VERSION" == "$MAIN_VERSION" ]]; then + echo "Error: Version in current branch is not bumped from main branch" + exit 1 + fi + - name: Start Redis run: | retries=3 From c1807c4b8203cdefe58d1257e69056da342e789e Mon Sep 17 00:00:00 2001 From: Shreya Shankar Date: Fri, 24 Nov 2023 18:55:52 -0600 Subject: [PATCH 22/37] Check that version is bumped up automatically --- .github/workflows/ci.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 29194c19..8ef64556 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -25,14 +25,14 @@ jobs: - name: Get current version from main branch id: main_version run: | - MAIN_VERSION=$(git show origin/main:pyproject.toml | grep -Po 'version = "\K[^"]+') + MAIN_VERSION=$(git show origin/main:pyproject.toml | awk -F '"' '/version =/ {print $2}') echo "Main branch version: $MAIN_VERSION" echo "MAIN_VERSION=$MAIN_VERSION" >> $GITHUB_ENV - name: Get version from current branch id: current_version run: | - CURRENT_VERSION=$(grep -Po 'version = "\K[^"]+' pyproject.toml) + CURRENT_VERSION=$(awk -F '"' '/version =/ {print $2}' pyproject.toml) echo "Current branch version: $CURRENT_VERSION" echo "CURRENT_VERSION=$CURRENT_VERSION" >> $GITHUB_ENV From da83fd8a53fb0a6f537fa11051885147afcbb608 Mon Sep 17 00:00:00 2001 From: Shreya Shankar Date: Fri, 24 Nov 2023 18:57:53 -0600 Subject: [PATCH 23/37] Check that version is bumped up automatically --- .github/workflows/ci.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8ef64556..8d7fcfc4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -25,14 +25,14 @@ jobs: - name: Get current version from main branch id: main_version run: | - MAIN_VERSION=$(git show origin/main:pyproject.toml | awk -F '"' '/version =/ {print $2}') + MAIN_VERSION=$(git show origin/main:pyproject.toml | grep '^version =' | awk -F '"' '{print $2}') echo "Main branch version: $MAIN_VERSION" echo "MAIN_VERSION=$MAIN_VERSION" >> $GITHUB_ENV - name: Get version from current branch id: current_version run: | - CURRENT_VERSION=$(awk -F '"' '/version =/ {print $2}' pyproject.toml) + CURRENT_VERSION=$(grep '^version =' pyproject.toml | awk -F '"' '{print $2}') echo "Current branch version: $CURRENT_VERSION" echo "CURRENT_VERSION=$CURRENT_VERSION" >> $GITHUB_ENV From 351290bc224124ff49ee5cc2deb800e60cb098b7 Mon Sep 17 00:00:00 2001 From: Shreya Shankar Date: Fri, 24 Nov 2023 18:59:25 -0600 Subject: [PATCH 24/37] Bump version --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 28fb4aa6..db9be934 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [tool.poetry] name = "motion-python" -version = "0.1.106" +version = "0.1.107" description = "A trigger-based framework for creating and executing ML pipelines." authors = ["Shreya Shankar "] readme = "README.md" From aac9cb38b09bdb46f59da6316a8bbf4e401aa96c Mon Sep 17 00:00:00 2001 From: Shreya Shankar Date: Fri, 24 Nov 2023 19:01:49 -0600 Subject: [PATCH 25/37] Give new major version 0.2 --- .github/workflows/cd.yml | 120 ++++++++++++++++++++++++++++++++++ .github/workflows/publish.yml | 67 ------------------- pyproject.toml | 2 +- 3 files changed, 121 insertions(+), 68 deletions(-) create mode 100644 .github/workflows/cd.yml delete mode 100644 .github/workflows/publish.yml diff --git a/.github/workflows/cd.yml b/.github/workflows/cd.yml new file mode 100644 index 00000000..b22e3b24 --- /dev/null +++ b/.github/workflows/cd.yml @@ -0,0 +1,120 @@ +# This file is autogenerated by maturin v0.14.17 +# To update, run +# +# maturin generate-ci github +# +name: CD + +on: + push: + branches: + - main + - master + tags: + - "*" + pull_request: + workflow_dispatch: + +permissions: + contents: read + +jobs: + linux: + runs-on: ubuntu-latest + strategy: + matrix: + target: [x86_64, x86, aarch64, armv7, s390x, ppc64le] + steps: + - uses: actions/checkout@v3 + - uses: actions/setup-python@v4 + with: + python-version: "3.10" + - name: Build wheels + uses: PyO3/maturin-action@v1 + with: + target: ${{ matrix.target }} + args: --release --out dist --find-interpreter + sccache: "true" + manylinux: auto + - name: Upload wheels + uses: actions/upload-artifact@v3 + with: + name: wheels + path: dist + + windows: + runs-on: windows-latest + strategy: + matrix: + target: [x64, x86] + steps: + - uses: actions/checkout@v3 + - uses: actions/setup-python@v4 + with: + python-version: "3.10" + architecture: ${{ matrix.target }} + - name: Build wheels + uses: PyO3/maturin-action@v1 + with: + target: ${{ matrix.target }} + args: --release --out dist --find-interpreter + sccache: "true" + - name: Upload wheels + uses: actions/upload-artifact@v3 + with: + name: wheels + path: dist + + macos: + runs-on: macos-latest + strategy: + matrix: + target: [x86_64, aarch64] + steps: + - uses: actions/checkout@v3 + - uses: actions/setup-python@v4 + with: + python-version: "3.10" + - name: Build wheels + uses: PyO3/maturin-action@v1 + with: + target: ${{ matrix.target }} + args: --release --out dist --find-interpreter + sccache: "true" + - name: Upload wheels + uses: actions/upload-artifact@v3 + with: + name: wheels + path: dist + + sdist: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + - name: Build sdist + uses: PyO3/maturin-action@v1 + with: + command: sdist + args: --out dist + - name: Upload sdist + uses: actions/upload-artifact@v3 + with: + name: wheels + path: dist + + release: + name: Release + runs-on: ubuntu-latest + if: "startsWith(github.ref, 'refs/tags/')" + needs: [linux, windows, macos, sdist] + steps: + - uses: actions/download-artifact@v3 + with: + name: wheels + - name: Publish to PyPI + uses: PyO3/maturin-action@v1 + env: + MATURIN_PYPI_TOKEN: ${{ secrets.PYPI_API_TOKEN }} + with: + command: upload + args: --skip-existing * diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml deleted file mode 100644 index d749adff..00000000 --- a/.github/workflows/publish.yml +++ /dev/null @@ -1,67 +0,0 @@ -name: publish - -on: - push: - branches: - - main - paths: - - "motion/**" - - "poetry.lock" - - "pyproject.toml" - -jobs: - publish-to-pypi: - runs-on: ubuntu-latest - if: ${{github.event.head_commit.author.name != 'github-actions[bot]' }} - steps: - - name: Print author - run: | - echo "Commit author name: ${{ github.event.head_commit.author.name }}" - echo "Commit author email: ${{ github.event.head_commit.author.email }}" - - name: Checkout code - uses: actions/checkout@v2 - with: - persist-credentials: false # otherwise, the token used is the GITHUB_TOKEN, instead of your personal token - fetch-depth: 0 # otherwise, you will failed to push refs to dest repo - - name: Configure Git - run: | - git config --global user.name "github-actions[bot]" - git config --global user.email "github-actions[bot]@users.noreply.github.com" - - name: Set up Python - uses: actions/setup-python@v2 - with: - python-version: 3.9 - - name: Install Poetry - uses: snok/install-poetry@v1 - - name: Bump version - id: bump_version - run: | - poetry version patch - - name: Build and publish - run: | - poetry config pypi-token.pypi ${{ secrets.PYPI_API_TOKEN }} - poetry build - poetry publish - git add pyproject.toml - git commit -m "Bump up version" - - name: Push changes - uses: ad-m/github-push-action@master - with: - github_token: ${{ secrets.BRANCH_PROTECTION_WORKAROUND }} - branch: main - - - name: Set release number - run: echo "RELEASE_NUMBER=$(poetry version --no-ansi | awk -F' ' '{print $2}')" >> $GITHUB_ENV - - name: Create release - id: create_release - uses: actions/create-release@v1 - env: - GITHUB_TOKEN: ${{ secrets.AUTORELEASE_TOKEN }} - RELEASE_NUMBER: ${{ env.RELEASE_NUMBER }} - with: - tag_name: v${{ env.RELEASE_NUMBER }} - release_name: Release ${{ env.RELEASE_NUMBER }} - body: | - An autorelease from the latest version of main. - draft: false - prerelease: true diff --git a/pyproject.toml b/pyproject.toml index db9be934..9f06a313 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [tool.poetry] name = "motion-python" -version = "0.1.107" +version = "0.2.0" description = "A trigger-based framework for creating and executing ML pipelines." authors = ["Shreya Shankar "] readme = "README.md" From 1b048152f0aa153f57c84ea1b8f02d380b80afb7 Mon Sep 17 00:00:00 2001 From: Shreya Shankar Date: Fri, 24 Nov 2023 19:19:53 -0600 Subject: [PATCH 26/37] Test pypi --- .github/workflows/cd.yml | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/.github/workflows/cd.yml b/.github/workflows/cd.yml index b22e3b24..276a7887 100644 --- a/.github/workflows/cd.yml +++ b/.github/workflows/cd.yml @@ -23,6 +23,7 @@ jobs: runs-on: ubuntu-latest strategy: matrix: + python-version: ["3.9", "3.10", "3.11"] target: [x86_64, x86, aarch64, armv7, s390x, ppc64le] steps: - uses: actions/checkout@v3 @@ -46,6 +47,7 @@ jobs: runs-on: windows-latest strategy: matrix: + python-version: ["3.9", "3.10", "3.11"] target: [x64, x86] steps: - uses: actions/checkout@v3 @@ -69,6 +71,7 @@ jobs: runs-on: macos-latest strategy: matrix: + python-version: ["3.9", "3.10", "3.11"] target: [x86_64, aarch64] steps: - uses: actions/checkout@v3 @@ -111,10 +114,18 @@ jobs: - uses: actions/download-artifact@v3 with: name: wheels - - name: Publish to PyPI - uses: PyO3/maturin-action@v1 + - name: Publish to TestPyPI + uses: messense/maturin-action@v1 env: MATURIN_PYPI_TOKEN: ${{ secrets.PYPI_API_TOKEN }} with: command: upload - args: --skip-existing * + args: --repository-url "https://test.pypi.org/legacy/" --skip-existing * + + # - name: Publish to PyPI + # uses: PyO3/maturin-action@v1 + # env: + # MATURIN_PYPI_TOKEN: ${{ secrets.PYPI_API_TOKEN }} + # with: + # command: upload + # args: --skip-existing * From 8bc16f2f27c4a26a34244582162ff242a7d42f99 Mon Sep 17 00:00:00 2001 From: Shreya Shankar Date: Fri, 24 Nov 2023 19:25:30 -0600 Subject: [PATCH 27/37] Properly handle python versions in CD --- .github/workflows/cd.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/cd.yml b/.github/workflows/cd.yml index 276a7887..a8edbd11 100644 --- a/.github/workflows/cd.yml +++ b/.github/workflows/cd.yml @@ -29,7 +29,7 @@ jobs: - uses: actions/checkout@v3 - uses: actions/setup-python@v4 with: - python-version: "3.10" + python-version: ${{ matrix.python-version }} - name: Build wheels uses: PyO3/maturin-action@v1 with: @@ -53,7 +53,7 @@ jobs: - uses: actions/checkout@v3 - uses: actions/setup-python@v4 with: - python-version: "3.10" + python-version: ${{ matrix.python-version }} architecture: ${{ matrix.target }} - name: Build wheels uses: PyO3/maturin-action@v1 @@ -77,7 +77,7 @@ jobs: - uses: actions/checkout@v3 - uses: actions/setup-python@v4 with: - python-version: "3.10" + python-version: ${{ matrix.python-version }} - name: Build wheels uses: PyO3/maturin-action@v1 with: From 5d5c554d535cbf2cc5f4deda3ae203d94bf44c66 Mon Sep 17 00:00:00 2001 From: Shreya Shankar Date: Fri, 24 Nov 2023 19:29:43 -0600 Subject: [PATCH 28/37] Properly handle python versions in CD --- .github/workflows/cd.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/cd.yml b/.github/workflows/cd.yml index a8edbd11..3d7074e8 100644 --- a/.github/workflows/cd.yml +++ b/.github/workflows/cd.yml @@ -34,7 +34,7 @@ jobs: uses: PyO3/maturin-action@v1 with: target: ${{ matrix.target }} - args: --release --out dist --find-interpreter + args: -i python${{ matrix.python-version }} --release --out dist --find-interpreter sccache: "true" manylinux: auto - name: Upload wheels From b34ef89ac18f6adc33e02a1d1ba39bd9e138f8ff Mon Sep 17 00:00:00 2001 From: Shreya Shankar Date: Fri, 24 Nov 2023 19:32:09 -0600 Subject: [PATCH 29/37] Properly handle python versions in CD --- .github/workflows/cd.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/cd.yml b/.github/workflows/cd.yml index 3d7074e8..0aa84d87 100644 --- a/.github/workflows/cd.yml +++ b/.github/workflows/cd.yml @@ -34,7 +34,7 @@ jobs: uses: PyO3/maturin-action@v1 with: target: ${{ matrix.target }} - args: -i python${{ matrix.python-version }} --release --out dist --find-interpreter + args: -i python${{ matrix.python-version }} --release --out dist sccache: "true" manylinux: auto - name: Upload wheels From 17749f68a33bca994702f6933c5fe94752169fbf Mon Sep 17 00:00:00 2001 From: Shreya Shankar Date: Fri, 24 Nov 2023 19:36:27 -0600 Subject: [PATCH 30/37] Get test pypi push to work --- .github/workflows/cd.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/cd.yml b/.github/workflows/cd.yml index 0aa84d87..d306ba0e 100644 --- a/.github/workflows/cd.yml +++ b/.github/workflows/cd.yml @@ -108,7 +108,6 @@ jobs: release: name: Release runs-on: ubuntu-latest - if: "startsWith(github.ref, 'refs/tags/')" needs: [linux, windows, macos, sdist] steps: - uses: actions/download-artifact@v3 From 4d59152fea8389f2e3e60e77b7cc9a1742bf241c Mon Sep 17 00:00:00 2001 From: Shreya Shankar Date: Fri, 24 Nov 2023 19:42:43 -0600 Subject: [PATCH 31/37] Fix formatting error --- .github/workflows/cd.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/cd.yml b/.github/workflows/cd.yml index d306ba0e..6b37448d 100644 --- a/.github/workflows/cd.yml +++ b/.github/workflows/cd.yml @@ -119,7 +119,7 @@ jobs: MATURIN_PYPI_TOKEN: ${{ secrets.PYPI_API_TOKEN }} with: command: upload - args: --repository-url "https://test.pypi.org/legacy/" --skip-existing * + args: --repository-url https://test.pypi.org/legacy/ --skip-existing * # - name: Publish to PyPI # uses: PyO3/maturin-action@v1 From 1c25ee0945ff00c09a885a8749ee8aeb76ba5912 Mon Sep 17 00:00:00 2001 From: Shreya Shankar Date: Fri, 24 Nov 2023 20:19:14 -0600 Subject: [PATCH 32/37] Try to get CD to work --- .github/workflows/cd.yml | 8 -------- 1 file changed, 8 deletions(-) diff --git a/.github/workflows/cd.yml b/.github/workflows/cd.yml index 6b37448d..90fcbe38 100644 --- a/.github/workflows/cd.yml +++ b/.github/workflows/cd.yml @@ -120,11 +120,3 @@ jobs: with: command: upload args: --repository-url https://test.pypi.org/legacy/ --skip-existing * - - # - name: Publish to PyPI - # uses: PyO3/maturin-action@v1 - # env: - # MATURIN_PYPI_TOKEN: ${{ secrets.PYPI_API_TOKEN }} - # with: - # command: upload - # args: --skip-existing * From 2e9a67c4e8f808cedb79d032a491702169082f27 Mon Sep 17 00:00:00 2001 From: Shreya Shankar Date: Fri, 24 Nov 2023 20:37:45 -0600 Subject: [PATCH 33/37] Try to fix CD --- .github/workflows/cd.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/cd.yml b/.github/workflows/cd.yml index 90fcbe38..003c374a 100644 --- a/.github/workflows/cd.yml +++ b/.github/workflows/cd.yml @@ -117,6 +117,7 @@ jobs: uses: messense/maturin-action@v1 env: MATURIN_PYPI_TOKEN: ${{ secrets.PYPI_API_TOKEN }} + MATURIN_REPOSITORY: "testpypi" with: command: upload - args: --repository-url https://test.pypi.org/legacy/ --skip-existing * + args: --skip-existing * From b4874cb9db8957676614da2ef068b1aab85c2583 Mon Sep 17 00:00:00 2001 From: Shreya Shankar Date: Sat, 25 Nov 2023 10:04:07 -0800 Subject: [PATCH 34/37] Add test pypi token --- .github/workflows/cd.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/cd.yml b/.github/workflows/cd.yml index 003c374a..7362029a 100644 --- a/.github/workflows/cd.yml +++ b/.github/workflows/cd.yml @@ -116,7 +116,7 @@ jobs: - name: Publish to TestPyPI uses: messense/maturin-action@v1 env: - MATURIN_PYPI_TOKEN: ${{ secrets.PYPI_API_TOKEN }} + MATURIN_PYPI_TOKEN: ${{ secrets.TEST_PYPI_API_TOKEN }} MATURIN_REPOSITORY: "testpypi" with: command: upload From c81b49128e623ce9ffd6caf0ab5b876154b780d4 Mon Sep 17 00:00:00 2001 From: Shreya Shankar Date: Sat, 25 Nov 2023 11:24:28 -0800 Subject: [PATCH 35/37] Fix cargo tests --- src/lib.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/lib.rs b/src/lib.rs index 4bf66114..a5dd2ea9 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -549,6 +549,7 @@ mod tests { 6381, 0, None, + None, ) .unwrap(); } @@ -563,6 +564,7 @@ mod tests { 6381, 0, None, + None, ); assert!(result.is_err()); } @@ -578,6 +580,7 @@ mod tests { 6381, 0, None, + None, ) .unwrap(); From 76bac356ff9068a5bff1f18e32562cacc904723e Mon Sep 17 00:00:00 2001 From: Shreya Shankar Date: Mon, 4 Dec 2023 23:21:46 -0700 Subject: [PATCH 36/37] Fix library --- Cargo.lock | 4 ++-- Cargo.toml | 4 ++-- Makefile | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 26918383..b27d878c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -128,8 +128,8 @@ dependencies = [ ] [[package]] -name = "motionstate" -version = "0.1.0" +name = "motion" +version = "0.2.0" dependencies = [ "bincode", "pyo3", diff --git a/Cargo.toml b/Cargo.toml index 59961aef..e82be5f4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] -name = "motionstate" -version = "0.1.0" +name = "motion" +version = "0.2.0" edition = "2021" # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html diff --git a/Makefile b/Makefile index d4bcf3b0..bda50399 100644 --- a/Makefile +++ b/Makefile @@ -20,4 +20,4 @@ docs: poetry run mkdocs serve build: - poetry run maturin develop + poetry run maturin develop --release From 7fbbae44becf42f553d58ae9a3c7c2df3e3fa849 Mon Sep 17 00:00:00 2001 From: Shreya Shankar Date: Fri, 8 Dec 2023 16:46:08 -0700 Subject: [PATCH 37/37] Pre check with cache ttl --- motion/execute.py | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/motion/execute.py b/motion/execute.py index 71ab03ca..02426d56 100644 --- a/motion/execute.py +++ b/motion/execute.py @@ -67,12 +67,11 @@ def __init__( self._instance_name.split("__")[1], self._redis_params.dict(), ) - + # If it is version 0, then call the init_state_func if self._state.get_version() == 0 and self._init_state_func is not None: update_dict = self._init_state_func(**self._init_state_params) self._state.flushUpdateDict(update_dict) - # Set up routes self._serve_routes: Dict[str, Route] = serve_routes @@ -109,14 +108,12 @@ def _connectToRedis(self) -> Tuple[RedisParams, redis.Redis]: r = redis.Redis(**param_dict) return rp, r - def _loadState(self, force_refresh: bool = True) -> State: # Clear state cache if force_refresh: self._state.clear_cache() return self._state - def setUp(self, **kwargs: Any) -> Dict[str, Any]: # Set up initial state if self._init_state_func is not None: @@ -249,10 +246,8 @@ def _updateState(self, new_state: Dict[str, Any]) -> None: if not isinstance(new_state, dict): raise TypeError("State should be a dict.") - self._state.flushUpdateDict(new_state) - def _enqueue_and_trigger_update( self, key: str, @@ -332,7 +327,6 @@ async def _async_enqueue_and_trigger_update( if flush_update: route = self._update_routes[key][update_udf_name] - try: state_update = route.run( state=self._state, @@ -352,7 +346,6 @@ async def _async_enqueue_and_trigger_update( "Error running update route in main process: " + str(e) ) - else: # Enqueue update @@ -461,7 +454,7 @@ def run( ) # Cache result - if value_hash: + if value_hash and self._cache_ttl > 0: cache_result_key = ( f"MOTION_RESULT:{self._instance_name}/{key}/{value_hash}" ) @@ -514,7 +507,7 @@ async def arun( props._serve_result = serve_result # Cache result - if value_hash: + if value_hash and self._cache_ttl > 0: cache_result_key = ( f"MOTION_RESULT:{self._instance_name}/{key}/{value_hash}" )