From 6815130118c4b66795dbe415626c59e8d590aaa3 Mon Sep 17 00:00:00 2001 From: Matt Mastracci Date: Thu, 21 Nov 2024 09:15:02 -0700 Subject: [PATCH] Bump pyo3 and tokio (#8008) The PyO3 API changes pretty dramatically, but we can use this opportunity to clean up a lot of the Python<->Rust interaction. The main pain is that `to_object` and `into_py` are now deprecated. --- Cargo.lock | 24 ++-- Cargo.toml | 2 +- .../edgeql-parser-python/src/errors.rs | 13 +- .../edgeql-parser-python/src/hash.rs | 10 +- .../edgeql-parser-python/src/keywords.rs | 51 +++----- .../edgeql-parser-python/src/lib.rs | 8 +- .../edgeql-parser-python/src/parser.rs | 111 ++++++++-------- .../edgeql-parser-python/src/position.rs | 3 +- .../edgeql-parser-python/src/pynormalize.rs | 71 +++++----- .../edgeql-parser-python/src/tokenizer.rs | 45 +++---- .../edgeql-parser-python/src/unpack.rs | 4 +- edb/edgeql-parser/src/into_python.rs | 117 ----------------- edb/edgeql-parser/src/lib.rs | 2 - edb/graphql-rewrite/src/lib.rs | 10 +- edb/graphql-rewrite/src/py_entry.rs | 52 ++++---- edb/graphql-rewrite/src/py_token.rs | 123 +++++++++--------- edb/server/conn_pool/src/python.rs | 55 +++++--- edb/server/http/src/python.rs | 51 ++++---- edb/server/pgrust/src/protocol/gen.rs | 2 +- edb/server/pgrust/src/python.rs | 34 ++--- 20 files changed, 332 insertions(+), 456 deletions(-) delete mode 100644 edb/edgeql-parser/src/into_python.rs diff --git a/Cargo.lock b/Cargo.lock index d82ddd3928b..70aab8897b1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1679,9 +1679,9 @@ dependencies = [ [[package]] name = "pyo3" -version = "0.22.2" +version = "0.23.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "831e8e819a138c36e212f3af3fd9eeffed6bf1510a805af35b0edee5ffa59433" +checksum = "7ebb0c0cc0de9678e53be9ccf8a2ab53045e6e3a8be03393ceccc5e7396ccb40" dependencies = [ "cfg-if", "indoc", @@ -1698,9 +1698,9 @@ dependencies = [ [[package]] name = "pyo3-build-config" -version = "0.22.2" +version = "0.23.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1e8730e591b14492a8945cdff32f089250b05f5accecf74aeddf9e8272ce1fa8" +checksum = "80e3ce69c4ec34476534b490e412b871ba03a82e35604c3dfb95fcb6bfb60c09" dependencies = [ "once_cell", "target-lexicon", @@ -1708,9 +1708,9 @@ dependencies = [ [[package]] name = "pyo3-ffi" -version = "0.22.2" +version = "0.23.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5e97e919d2df92eb88ca80a037969f44e5e70356559654962cbb3316d00300c6" +checksum = "3b09f311c76b36dfd6dd6f7fa6f9f18e7e46a1c937110d283e80b12ba2468a75" dependencies = [ "libc", "pyo3-build-config", @@ -1718,9 +1718,9 @@ dependencies = [ [[package]] name = "pyo3-macros" -version = "0.22.2" +version = "0.23.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "eb57983022ad41f9e683a599f2fd13c3664d7063a3ac5714cae4b7bee7d3f206" +checksum = "fd4f74086536d1e1deaff99ec0387481fb3325c82e4e48be0e75ab3d3fcb487a" dependencies = [ "proc-macro2", "pyo3-macros-backend", @@ -1730,9 +1730,9 @@ dependencies = [ [[package]] name = "pyo3-macros-backend" -version = "0.22.2" +version = "0.23.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ec480c0c51ddec81019531705acac51bcdbeae563557c982aa8263bb96880372" +checksum = "9e77dfeb76b32bbf069144a5ea0a36176ab59c8db9ce28732d0f06f096bbfbc8" dependencies = [ "heck", "proc-macro2", @@ -2461,9 +2461,9 @@ checksum = "1f3ccbac311fea05f86f61904b462b55fb3df8837a366dfc601a0161d0532f20" [[package]] name = "tokio" -version = "1.39.3" +version = "1.41.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9babc99b9923bfa4804bd74722ff02c0381021eafa4db9949217e3be8e84fff5" +checksum = "22cfb5bee7a6a52939ca9224d6ac897bb669134078daa8735560897f69de4d33" dependencies = [ "backtrace", "bytes", diff --git a/Cargo.toml b/Cargo.toml index 9ea22ec1dd9..d5bb17fc60d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -11,7 +11,7 @@ members = [ resolver = "2" [workspace.dependencies] -pyo3 = { version = "0.22.2", features = ["extension-module", "serde"] } +pyo3 = { version = "0.23.1", features = ["extension-module", "serde"] } tokio = { version = "1", features = ["rt", "rt-multi-thread", "macros", "time", "sync", "net", "io-util"] } tracing = "0.1.40" tracing-subscriber = "0.3.18" diff --git a/edb/edgeql-parser/edgeql-parser-python/src/errors.rs b/edb/edgeql-parser/edgeql-parser-python/src/errors.rs index 268fdf1619d..a62e6b48bd2 100644 --- a/edb/edgeql-parser/edgeql-parser-python/src/errors.rs +++ b/edb/edgeql-parser/edgeql-parser-python/src/errors.rs @@ -29,16 +29,17 @@ impl ParserResult { let mut buf = vec![0u8]; // type and version bincode::serialize_into(&mut buf, &rv) .map_err(|e| PyValueError::new_err(format!("Failed to pack: {e}")))?; - Ok(PyBytes::new_bound(py, buf.as_slice()).into()) + Ok(PyBytes::new(py, buf.as_slice()).into()) } } -pub fn parser_error_into_tuple(py: Python, error: Error) -> PyObject { +pub fn parser_error_into_tuple( + error: &Error, +) -> (&str, (u64, u64), Option<&String>, Option<&String>) { ( - error.message, + &error.message, (error.span.start, error.span.end), - error.hint, - error.details, + error.hint.as_ref(), + error.details.as_ref(), ) - .into_py(py) } diff --git a/edb/edgeql-parser/edgeql-parser-python/src/hash.rs b/edb/edgeql-parser/edgeql-parser-python/src/hash.rs index 9139c0d09f1..f52a2b69170 100644 --- a/edb/edgeql-parser/edgeql-parser-python/src/hash.rs +++ b/edb/edgeql-parser/edgeql-parser-python/src/hash.rs @@ -1,4 +1,4 @@ -use std::cell::RefCell; +use std::sync::RwLock; use edgeql_parser::hash; use pyo3::{exceptions::PyRuntimeError, prelude::*, types::PyString}; @@ -7,7 +7,7 @@ use crate::errors::SyntaxError; #[pyclass] pub struct Hasher { - _hasher: RefCell>, + _hasher: RwLock>, } #[pymethods] @@ -16,13 +16,13 @@ impl Hasher { fn start_migration(parent_id: &Bound) -> PyResult { let hasher = hash::Hasher::start_migration(parent_id.to_str()?); Ok(Hasher { - _hasher: RefCell::new(Some(hasher)), + _hasher: RwLock::new(Some(hasher)), }) } fn add_source(&self, py: Python, data: &Bound) -> PyResult { let text = data.to_str()?; - let mut cell = self._hasher.borrow_mut(); + let mut cell = self._hasher.write().unwrap(); let hasher = cell .as_mut() .ok_or_else(|| PyRuntimeError::new_err(("cannot add source after finish",)))?; @@ -36,7 +36,7 @@ impl Hasher { } fn make_migration_id(&self) -> PyResult { - let mut cell = self._hasher.borrow_mut(); + let mut cell = self._hasher.write().unwrap(); let hasher = cell .take() .ok_or_else(|| PyRuntimeError::new_err(("cannot do migration id twice",)))?; diff --git a/edb/edgeql-parser/edgeql-parser-python/src/keywords.rs b/edb/edgeql-parser/edgeql-parser-python/src/keywords.rs index 02e244b670c..7c7d5968ec0 100644 --- a/edb/edgeql-parser/edgeql-parser-python/src/keywords.rs +++ b/edb/edgeql-parser/edgeql-parser-python/src/keywords.rs @@ -1,46 +1,35 @@ -use pyo3::{ - prelude::*, - types::{PyList, PyString}, -}; +use pyo3::{prelude::*, types::PyFrozenSet}; use edgeql_parser::keywords; pub struct AllKeywords { - pub current: PyObject, - pub future: PyObject, - pub unreserved: PyObject, - pub partial: PyObject, + pub current: Py, + pub future: Py, + pub unreserved: Py, + pub partial: Py, } pub fn get_keywords(py: Python) -> PyResult { - let intern = py.import_bound("sys")?.getattr("intern")?; - let frozen = py.import_bound("builtins")?.getattr("frozenset")?; + let intern = py.import("sys")?.getattr("intern")?; - let current = prepare_keywords(py, keywords::CURRENT_RESERVED_KEYWORDS.iter(), &intern)?; - let unreserved = prepare_keywords(py, keywords::UNRESERVED_KEYWORDS.iter(), &intern)?; - let future = prepare_keywords(py, keywords::FUTURE_RESERVED_KEYWORDS.iter(), &intern)?; - let partial = prepare_keywords(py, keywords::PARTIAL_RESERVED_KEYWORDS.iter(), &intern)?; Ok(AllKeywords { - current: frozen - .call((PyList::new_bound(py, ¤t),), None)? - .into(), - unreserved: frozen - .call((PyList::new_bound(py, &unreserved),), None)? - .into(), - future: frozen.call((PyList::new_bound(py, &future),), None)?.into(), - partial: frozen - .call((PyList::new_bound(py, &partial),), None)? - .into(), + current: prepare_keywords(py, &keywords::CURRENT_RESERVED_KEYWORDS, &intern)?, + unreserved: prepare_keywords(py, &keywords::UNRESERVED_KEYWORDS, &intern)?, + future: prepare_keywords(py, &keywords::FUTURE_RESERVED_KEYWORDS, &intern)?, + partial: prepare_keywords(py, &keywords::PARTIAL_RESERVED_KEYWORDS, &intern)?, }) } -fn prepare_keywords<'py, I: Iterator>( +fn prepare_keywords<'a, 'py, I: IntoIterator>( py: Python<'py>, keyword_set: I, - intern: &'py Bound<'py, PyAny>, -) -> Result>, PyErr> { - keyword_set - .cloned() - .map(|s: &str| intern.call((PyString::new_bound(py, s),), None)) - .collect() + intern: &Bound<'py, PyAny>, +) -> PyResult> { + PyFrozenSet::new( + py, + keyword_set + .into_iter() + .map(|s| intern.call((&s,), None).unwrap()), + ) + .map(|o| o.unbind()) } diff --git a/edb/edgeql-parser/edgeql-parser-python/src/lib.rs b/edb/edgeql-parser/edgeql-parser-python/src/lib.rs index b36c0ddc9f6..d1fbcad8f30 100644 --- a/edb/edgeql-parser/edgeql-parser-python/src/lib.rs +++ b/edb/edgeql-parser/edgeql-parser-python/src/lib.rs @@ -14,8 +14,8 @@ use pyo3::prelude::*; /// Rust bindings to the edgeql-parser crate #[pymodule] fn _edgeql_parser(py: Python, m: &Bound) -> PyResult<()> { - m.add("SyntaxError", py.get_type_bound::())?; - m.add("ParserResult", py.get_type_bound::())?; + m.add("SyntaxError", py.get_type::())?; + m.add("ParserResult", py.get_type::())?; m.add_class::()?; @@ -36,7 +36,7 @@ fn _edgeql_parser(py: Python, m: &Bound) -> PyResult<()> { m.add_class::()?; m.add_function(wrap_pyfunction!(position::offset_of_line, m)?)?; - m.add("SourcePoint", py.get_type_bound::())?; + m.add("SourcePoint", py.get_type::())?; m.add_class::()?; m.add_function(wrap_pyfunction!(tokenizer::tokenize, m)?)?; @@ -44,7 +44,7 @@ fn _edgeql_parser(py: Python, m: &Bound) -> PyResult<()> { m.add_function(wrap_pyfunction!(unpack::unpack, m)?)?; - tokenizer::fini_module(py, m); + tokenizer::fini_module(m); Ok(()) } diff --git a/edb/edgeql-parser/edgeql-parser-python/src/parser.rs b/edb/edgeql-parser/edgeql-parser-python/src/parser.rs index 678612c30df..0f572ddd59b 100644 --- a/edb/edgeql-parser/edgeql-parser-python/src/parser.rs +++ b/edb/edgeql-parser/edgeql-parser-python/src/parser.rs @@ -3,10 +3,10 @@ use once_cell::sync::OnceCell; use edgeql_parser::parser; use pyo3::exceptions::{PyAssertionError, PyValueError}; use pyo3::prelude::*; -use pyo3::types::{PyList, PyString, PyTuple}; +use pyo3::types::{PyList, PyString}; use crate::errors::{parser_error_into_tuple, ParserResult}; -use crate::pynormalize::value_to_py_object; +use crate::pynormalize::TokenizerValue; use crate::tokenizer::OpaqueToken; #[pyfunction] @@ -14,7 +14,7 @@ pub fn parse( py: Python, start_token_name: &Bound, tokens: PyObject, -) -> PyResult<(ParserResult, PyObject)> { +) -> PyResult<(ParserResult, &'static Py)> { let start_token_name = start_token_name.to_string(); let (spec, productions) = get_spec()?; @@ -24,28 +24,22 @@ pub fn parse( let context = parser::Context::new(spec); let (cst, errors) = parser::parse(&tokens, &context); - let cst = cst.map(|c| to_py_cst(&c, py)).transpose()?; - - let errors = errors - .into_iter() - .map(|e| parser_error_into_tuple(py, e)) - .collect::>(); - let errors = PyList::new_bound(py, &errors); + let errors = PyList::new(py, errors.iter().map(|e| parser_error_into_tuple(e)))?; let res = ParserResult { - out: cst.into_py(py), + out: cst.as_ref().map(ParserCSTNode).into_pyobject(py)?.unbind(), errors: errors.into(), }; - Ok((res, productions.to_object(py))) + Ok((res, productions)) } #[pyclass] pub struct CSTNode { #[pyo3(get)] - production: PyObject, + production: Option>, #[pyo3(get)] - terminal: PyObject, + terminal: Option>, } #[pyclass] @@ -136,56 +130,55 @@ pub fn save_spec(spec_json: &Bound, dst: &Bound) -> PyResult fn load_productions(py: Python<'_>, spec: &parser::Spec) -> PyResult { let grammar_name = "edb.edgeql.parser.grammar.start"; - let grammar_mod = py.import_bound(grammar_name)?; + let grammar_mod = py.import(grammar_name)?; let load_productions = py - .import_bound("edb.common.parsing")? + .import("edb.common.parsing")? .getattr("load_spec_productions")?; - let production_names: Vec<_> = spec - .production_names - .iter() - .map(|(a, b)| PyTuple::new_bound(py, [a, b])) - .collect(); - - let productions = load_productions.call((production_names, grammar_mod), None)?; + let productions = load_productions.call((&spec.production_names, grammar_mod), None)?; Ok(productions.into()) } -fn to_py_cst<'a>(cst: &'a parser::CSTNode<'a>, py: Python) -> PyResult { - Ok(match cst { - parser::CSTNode::Empty => CSTNode { - production: py.None(), - terminal: py.None(), - }, - parser::CSTNode::Terminal(token) => CSTNode { - production: py.None(), - terminal: Terminal { - text: token.text.clone(), - value: if let Some(val) = &token.value { - value_to_py_object(py, val)? - } else { - py.None() - }, - start: token.span.start, - end: token.span.end, - } - .into_py(py), - }, - parser::CSTNode::Production(prod) => CSTNode { - production: Production { - id: prod.id, - args: PyList::new_bound( +/// Newtype required to define a trait for a foreign type. +struct ParserCSTNode<'a>(&'a parser::CSTNode<'a>); + +impl<'a, 'py> IntoPyObject<'py> for ParserCSTNode<'a> { + type Target = CSTNode; + type Output = Bound<'py, Self::Target>; + type Error = PyErr; + + fn into_pyobject(self, py: Python<'py>) -> PyResult { + let res = match self.0 { + parser::CSTNode::Empty => CSTNode { + production: None, + terminal: None, + }, + parser::CSTNode::Terminal(token) => CSTNode { + production: None, + terminal: Some(Py::new( py, - prod.args - .iter() - .map(|a| to_py_cst(a, py).map(|x| x.into_py(py))) - .collect::>>()? - .as_slice(), - ) - .into(), - } - .into_py(py), - terminal: py.None(), - }, - }) + Terminal { + text: token.text.clone(), + value: (token.value.as_ref()) + .map(TokenizerValue) + .into_pyobject(py)? + .unbind(), + start: token.span.start, + end: token.span.end, + }, + )?), + }, + parser::CSTNode::Production(prod) => CSTNode { + production: Some(Py::new( + py, + Production { + id: prod.id, + args: PyList::new(py, prod.args.iter().map(ParserCSTNode))?.into(), + }, + )?), + terminal: None, + }, + }; + Ok(Py::new(py, res)?.bind(py).clone()) + } } diff --git a/edb/edgeql-parser/edgeql-parser-python/src/position.rs b/edb/edgeql-parser/edgeql-parser-python/src/position.rs index 42b5bc54407..f977cc3ee78 100644 --- a/edb/edgeql-parser/edgeql-parser-python/src/position.rs +++ b/edb/edgeql-parser/edgeql-parser-python/src/position.rs @@ -25,7 +25,8 @@ impl SourcePoint { .into_iter() .map(|_position| SourcePoint { _position }) .collect::>() - .into_py(py)) + .into_pyobject(py)? + .unbind()) } #[getter] diff --git a/edb/edgeql-parser/edgeql-parser-python/src/pynormalize.rs b/edb/edgeql-parser/edgeql-parser-python/src/pynormalize.rs index ce56df919a4..e4db89f7561 100644 --- a/edb/edgeql-parser/edgeql-parser-python/src/pynormalize.rs +++ b/edb/edgeql-parser/edgeql-parser-python/src/pynormalize.rs @@ -8,7 +8,7 @@ use edgedb_protocol::model::{BigInt, Decimal}; use edgeql_parser::tokenizer::Value; use pyo3::exceptions::{PyAssertionError, PyValueError}; use pyo3::prelude::*; -use pyo3::types::{PyBytes, PyDict, PyFloat, PyList, PyLong, PyString}; +use pyo3::types::{PyBytes, PyDict, PyFloat, PyInt, PyList, PyString}; use crate::errors::SyntaxError; use crate::normalize::{normalize as _normalize, Error, PackedEntry, Variable}; @@ -55,20 +55,16 @@ pub struct Entry { impl Entry { pub fn new(py: Python, entry: crate::normalize::Entry) -> PyResult { - let blobs = serialize_all(py, &entry.variables).map_err(PyAssertionError::new_err)?; - let counts: Vec<_> = entry - .variables - .iter() - .map(|x| x.len().into_py(py)) - .collect(); + let blobs = serialize_all(py, &entry.variables)?; + let counts = entry.variables.iter().map(|x| x.len()); Ok(Entry { - key: PyBytes::new_bound(py, &entry.hash[..]).into(), - tokens: tokens_to_py(py, entry.tokens.clone())?, + key: PyBytes::new(py, &entry.hash[..]).into(), + tokens: tokens_to_py(py, entry.tokens.clone())?.into_any(), extra_blobs: blobs.into(), extra_named: entry.named_args, first_extra: entry.first_arg, - extra_counts: PyList::new_bound(py, &counts[..]).into(), + extra_counts: PyList::new(py, counts)?.into(), entry_pack: entry.into(), }) } @@ -77,10 +73,10 @@ impl Entry { #[pymethods] impl Entry { fn get_variables(&self, py: Python) -> PyResult { - let vars = PyDict::new_bound(py); + let vars = PyDict::new(py); let first = match self.first_extra { Some(first) => first, - None => return Ok(vars.to_object(py)), + None => return Ok(vars.unbind().into_any()), }; for (idx, var) in self.entry_pack.variables.iter().flatten().enumerate() { let s = if self.extra_named { @@ -88,17 +84,17 @@ impl Entry { } else { (first + idx).to_string() }; - vars.set_item(s.into_py(py), value_to_py_object(py, &var.value)?)?; + vars.set_item(s, TokenizerValue(&var.value))?; } - Ok(vars.to_object(py)) + Ok(vars.unbind().into_any()) } fn pack(&self, py: Python) -> PyResult { let mut buf = vec![1u8]; // type and version bincode::serialize_into(&mut buf, &self.entry_pack) .map_err(|e| PyValueError::new_err(format!("Failed to pack: {e}")))?; - Ok(PyBytes::new_bound(py, buf.as_slice()).into()) + Ok(PyBytes::new(py, buf.as_slice()).into()) } } @@ -167,28 +163,35 @@ pub fn serialize_extra(variables: &[Variable]) -> Result { pub fn serialize_all<'a>( py: Python<'a>, variables: &[Vec], -) -> Result, String> { +) -> PyResult> { let mut buf = Vec::with_capacity(variables.len()); for vars in variables { - let bytes = serialize_extra(vars)?; - buf.push(PyBytes::new_bound(py, &bytes)); + let bytes = serialize_extra(vars).map_err(PyAssertionError::new_err)?; + buf.push(PyBytes::new(py, &bytes)); } - Ok(PyList::new_bound(py, &buf)) + PyList::new(py, &buf) } -pub fn value_to_py_object(py: Python, val: &Value) -> PyResult { - Ok(match val { - Value::Int(v) => v.into_py(py), - Value::String(v) => v.into_py(py), - Value::Float(v) => v.into_py(py), - Value::BigInt(v) => py - .get_type_bound::() - .call((v, 16.into_py(py)), None)? - .into(), - Value::Decimal(v) => py - .get_type_bound::() - .call((v.to_string(),), None)? - .into(), - Value::Bytes(v) => PyBytes::new_bound(py, v).into(), - }) +/// Newtype required to define a trait for a foreign type. +pub struct TokenizerValue<'a>(pub &'a Value); + +impl<'py> IntoPyObject<'py> for TokenizerValue<'py> { + type Target = PyAny; + type Output = Bound<'py, Self::Target>; + type Error = PyErr; + + fn into_pyobject(self, py: Python<'py>) -> PyResult { + let res = match self.0 { + Value::Int(v) => v.into_pyobject(py)?.into_any(), + Value::String(v) => v.into_pyobject(py)?.into_any(), + Value::Float(v) => v.into_pyobject(py)?.into_any(), + Value::BigInt(v) => py.get_type::().call((v, 16), None)?, + Value::Decimal(v) => py + .get_type::() + .call((v.to_string(),), None)? + .into_any(), + Value::Bytes(v) => PyBytes::new(py, v).into_any(), + }; + Ok(res) + } } diff --git a/edb/edgeql-parser/edgeql-parser-python/src/tokenizer.rs b/edb/edgeql-parser/edgeql-parser-python/src/tokenizer.rs index 54a2b5e0f47..67736b3d2fe 100644 --- a/edb/edgeql-parser/edgeql-parser-python/src/tokenizer.rs +++ b/edb/edgeql-parser/edgeql-parser-python/src/tokenizer.rs @@ -10,16 +10,16 @@ use crate::errors::{parser_error_into_tuple, ParserResult}; pub fn tokenize(py: Python, s: &Bound) -> PyResult { let data = s.to_string(); - let mut token_stream = Tokenizer::new(&data[..]).validated_values().with_eof(); + let token_stream = Tokenizer::new(&data[..]).validated_values().with_eof(); - let mut tokens: Vec<_> = Vec::new(); - let mut errors: Vec<_> = Vec::new(); + let mut tokens = vec![]; + let mut errors = vec![]; - for res in &mut token_stream { + for res in token_stream.into_iter() { match res { Ok(token) => tokens.push(token), Err(e) => { - errors.push(parser_error_into_tuple(py, e)); + errors.push(parser_error_into_tuple(&e).into_pyobject(py)?); // TODO: fix tokenizer to skip bad tokens and continue break; @@ -27,14 +27,13 @@ pub fn tokenize(py: Python, s: &Bound) -> PyResult { } } - let tokens = tokens_to_py(py, tokens)?; + let out = tokens_to_py(py, tokens)? + .into_pyobject(py)? + .unbind() + .into_any(); + let errors = PyList::new(py, errors)?.unbind().into_any(); - let errors = PyList::new_bound(py, errors.as_slice()).into_py(py); - - Ok(ParserResult { - out: tokens.into_py(py), - errors, - }) + Ok(ParserResult { out, errors }) } // An opaque wrapper around [edgeql_parser::tokenizer::Token]. @@ -54,20 +53,18 @@ impl OpaqueToken { .map_err(|e| PyValueError::new_err(format!("Failed to reduce: {e}")))?; let tok = get_unpickle_token_fn(py); - Ok((tok, (PyBytes::new_bound(py, &data).to_object(py),))) + Ok((tok, (PyBytes::new(py, &data).unbind().into_any(),))) } } -pub fn tokens_to_py(py: Python<'_>, rust_tokens: Vec) -> PyResult { - let mut buf = Vec::with_capacity(rust_tokens.len()); - for tok in rust_tokens { - let py_tok = OpaqueToken { +pub fn tokens_to_py(py: Python<'_>, rust_tokens: Vec) -> PyResult> { + Ok(PyList::new( + py, + rust_tokens.into_iter().map(|tok| OpaqueToken { inner: tok.cloned(), - }; - - buf.push(py_tok.into_py(py)); - } - Ok(PyList::new_bound(py, &buf[..]).into_py(py)) + }), + )? + .unbind()) } /// To support pickle serialization of OpaqueTokens, we need to provide a @@ -79,10 +76,10 @@ pub fn tokens_to_py(py: Python<'_>, rust_tokens: Vec) -> PyResult = OnceCell::new(); -pub fn fini_module(py: Python, m: &Bound) { +pub fn fini_module(m: &Bound) { let _unpickle_token = m.getattr("unpickle_token").unwrap(); FN_UNPICKLE_TOKEN - .set(_unpickle_token.to_object(py)) + .set(_unpickle_token.unbind()) .expect("module is already initialized"); } diff --git a/edb/edgeql-parser/edgeql-parser-python/src/unpack.rs b/edb/edgeql-parser/edgeql-parser-python/src/unpack.rs index 11c178d44a4..ad17edf80e1 100644 --- a/edb/edgeql-parser/edgeql-parser-python/src/unpack.rs +++ b/edb/edgeql-parser/edgeql-parser-python/src/unpack.rs @@ -14,13 +14,13 @@ pub fn unpack(py: Python<'_>, serialized: &Bound) -> PyResult 0u8 => { let tokens: Vec = bincode::deserialize(&buf[1..]) .map_err(|e| PyValueError::new_err(format!("{e}")))?; - tokens_to_py(py, tokens) + Ok(tokens_to_py(py, tokens)?.into_any()) } 1u8 => { let pack: PackedEntry = bincode::deserialize(&buf[1..]) .map_err(|e| PyValueError::new_err(format!("Failed to unpack: {e}")))?; let entry = Entry::new(py, pack.into())?; - Ok(entry.into_py(py)) + entry.into_pyobject(py).map(|e| e.unbind().into_any()) } _ => Err(PyValueError::new_err(format!( "Invalid type/version byte: {}", diff --git a/edb/edgeql-parser/src/into_python.rs b/edb/edgeql-parser/src/into_python.rs deleted file mode 100644 index e2f4f4c4335..00000000000 --- a/edb/edgeql-parser/src/into_python.rs +++ /dev/null @@ -1,117 +0,0 @@ -#![cfg(never)] // TODO: migrate cpython-rust to pyo3 -use indexmap::IndexMap; - -use cpython::{ - PyBytes, PyDict, PyList, PyObject, PyResult, PyTuple, Python, PythonObject, ToPyObject, -}; - -/// Convert into a Python object. -/// -/// Primitives (i64, String, Option, Vec) have this trait implemented with -/// calls to [cpython]. -/// -/// Structs have this trait derived to collect all their properties into a -/// [PyDict] and call constructor of the AST node. -/// -/// Enums represent either enums, union types or child classes and thus have -/// three different derive implementations. -/// -/// See [edgeql_parser_derive] crate. -pub trait IntoPython: Sized { - fn into_python(self, py: Python<'_>, parent_kw_args: Option) -> PyResult; -} - -impl IntoPython for String { - fn into_python(self, py: Python<'_>, _: Option) -> PyResult { - Ok(self.to_py_object(py).into_object()) - } -} - -impl IntoPython for i64 { - fn into_python(self, py: Python<'_>, _: Option) -> PyResult { - Ok(self.to_py_object(py).into_object()) - } -} - -impl IntoPython for f64 { - fn into_python(self, py: Python<'_>, _: Option) -> PyResult { - Ok(self.to_py_object(py).into_object()) - } -} - -impl IntoPython for bool { - fn into_python(self, py: Python<'_>, _: Option) -> PyResult { - Ok(if self { py.True() } else { py.False() }.into_object()) - } -} - -impl IntoPython for Vec { - fn into_python(self, py: Python<'_>, _: Option) -> PyResult { - let mut elements = Vec::new(); - for x in self { - elements.push(x.into_python(py, None)?); - } - Ok(PyList::new_bound(py, elements.as_slice()).into_object()) - } -} - -impl IntoPython for Option { - fn into_python(self, py: Python<'_>, _: Option) -> PyResult { - if let Some(value) = self { - value.into_python(py, None) - } else { - Ok(py.None()) - } - } -} - -impl IntoPython for Box { - fn into_python(self, py: Python<'_>, _: Option) -> PyResult { - (*self).into_python(py, None) - } -} - -impl IntoPython for (T1, T2) { - fn into_python(self, py: Python<'_>, _: Option) -> PyResult { - let mut elements = Vec::new(); - elements.push(self.0.into_python(py, None)?); - elements.push(self.1.into_python(py, None)?); - Ok(PyTuple::new_bound(py, elements.as_slice()).into_object()) - } -} - -impl IntoPython for IndexMap { - fn into_python(self, py: Python<'_>, _: Option) -> PyResult { - let dict = PyDict::new_bound(py); - for (key, value) in self { - let key = key.into_python(py, None)?; - let value = value.into_python(py, None)?; - dict.set_item(py, key, value)?; - } - Ok(dict.into_object()) - } -} - -impl IntoPython for Vec { - fn into_python(self, py: Python<'_>, _: Option) -> PyResult { - Ok(PyBytes::new_bound(py, self.as_slice()).into_object()) - } -} - -impl IntoPython for () { - fn into_python(self, py: Python<'_>, _: Option) -> PyResult { - Ok(py.None().into_object()) - } -} - -pub fn init_ast_class( - py: Python, - class_name: &'static str, - kw_args: PyDict, -) -> Result { - let locals = PyDict::new_bound(py); - locals.set_item(py, "kw_args", kw_args)?; - - let code = format!("qlast.{class_name}(**kw_args)"); - py.eval(&code, None, Some(&locals)) -} diff --git a/edb/edgeql-parser/src/lib.rs b/edb/edgeql-parser/src/lib.rs index 36841764a4a..01cd1304b72 100644 --- a/edb/edgeql-parser/src/lib.rs +++ b/edb/edgeql-parser/src/lib.rs @@ -2,8 +2,6 @@ pub mod ast; pub mod expr; pub mod hash; pub mod helpers; -#[cfg(feature = "python")] -pub mod into_python; pub mod keywords; pub mod parser; pub mod position; diff --git a/edb/graphql-rewrite/src/lib.rs b/edb/graphql-rewrite/src/lib.rs index 17cab94b874..d1de0ca4f4c 100644 --- a/edb/graphql-rewrite/src/lib.rs +++ b/edb/graphql-rewrite/src/lib.rs @@ -16,11 +16,11 @@ use pyo3::{prelude::*, types::PyString}; fn _graphql_rewrite(py: Python, m: &Bound) -> PyResult<()> { m.add_function(wrap_pyfunction!(py_rewrite, m)?)?; m.add_class::()?; - m.add("LexingError", py.get_type_bound::())?; - m.add("SyntaxError", py.get_type_bound::())?; - m.add("NotFoundError", py.get_type_bound::())?; - m.add("AssertionError", py.get_type_bound::())?; - m.add("QueryError", py.get_type_bound::())?; + m.add("LexingError", py.get_type::())?; + m.add("SyntaxError", py.get_type::())?; + m.add("NotFoundError", py.get_type::())?; + m.add("AssertionError", py.get_type::())?; + m.add("QueryError", py.get_type::())?; Ok(()) } diff --git a/edb/graphql-rewrite/src/py_entry.rs b/edb/graphql-rewrite/src/py_entry.rs index 05e082b8e20..ff8cc6ff766 100644 --- a/edb/graphql-rewrite/src/py_entry.rs +++ b/edb/graphql-rewrite/src/py_entry.rs @@ -1,5 +1,5 @@ use pyo3::prelude::*; -use pyo3::types::{PyDict, PyList, PyLong, PyString, PyTuple, PyType}; +use pyo3::types::{PyDict, PyInt, PyList, PyString, PyTuple, PyType}; use edb_graphql_parser::position::Pos; @@ -22,24 +22,23 @@ pub struct Entry { #[pymethods] impl Entry { - fn tokens(&self, py: Python, kinds: PyObject) -> PyResult { + fn tokens<'py>(&self, py: Python<'py>, kinds: PyObject) -> PyResult> { py_token::convert_tokens(py, &self._tokens, &self._end_pos, kinds) } } pub fn convert_entry(py: Python<'_>, entry: rewrite::Entry) -> PyResult { // import decimal - let decimal_cls = PyModule::import_bound(py, "decimal")?.getattr("Decimal")?; + let decimal_cls = PyModule::import(py, "decimal")?.getattr("Decimal")?; - let vars = PyDict::new_bound(py); - let substitutions = PyDict::new_bound(py); + let vars = PyDict::new(py); + let substitutions = PyDict::new(py); for (idx, var) in entry.variables.iter().enumerate() { - let s = format!("_edb_arg__{}", idx).to_object(py); - - vars.set_item(s.clone_ref(py), value_to_py(py, &var.value, &decimal_cls)?)?; + let s = format!("_edb_arg__{}", idx).into_pyobject(py)?; + vars.set_item(&s, value_to_py(py, &var.value, &decimal_cls)?)?; substitutions.set_item( - s.clone_ref(py), + s, ( &var.token.value, var.token.position.map(|x| x.line), @@ -48,20 +47,13 @@ pub fn convert_entry(py: Python<'_>, entry: rewrite::Entry) -> PyResult { )?; } for (name, var) in &entry.defaults { - vars.set_item(name.into_py(py), value_to_py(py, &var.value, &decimal_cls)?)? + vars.set_item(name, value_to_py(py, &var.value, &decimal_cls)?)? } - let key_vars = PyList::new_bound( - py, - entry - .key_vars - .iter() - .map(|v| v.into_py(py)) - .collect::>(), - ); + let key_vars = PyList::new(py, entry.key_vars)?; Ok(Entry { - key: PyString::new_bound(py, &entry.key).into(), + key: PyString::new(py, &entry.key).into(), key_vars: key_vars.into(), - variables: vars.into_py(py), + variables: vars.into_pyobject(py)?.unbind().into_any(), substitutions: substitutions.into(), _tokens: entry.tokens, _end_pos: entry.end_pos, @@ -70,16 +62,16 @@ pub fn convert_entry(py: Python<'_>, entry: rewrite::Entry) -> PyResult { fn value_to_py(py: Python, value: &Value, decimal_cls: &Bound) -> PyResult { let v = match value { - Value::Str(ref v) => PyString::new_bound(py, v).into(), - Value::Int32(v) => v.into_py(py), - Value::Int64(v) => v.into_py(py), + Value::Str(ref v) => PyString::new(py, v).into_any(), + Value::Int32(v) => v.into_pyobject(py)?.into_any(), + Value::Int64(v) => v.into_pyobject(py)?.into_any(), Value::Decimal(v) => decimal_cls - .call(PyTuple::new_bound(py, &[v.into_py(py)]), None)? - .into(), - Value::BigInt(ref v) => PyType::new_bound::(py) - .call(PyTuple::new_bound(py, &[v.into_py(py)]), None)? - .into(), - Value::Boolean(b) => b.into_py(py), + .call(PyTuple::new(py, &[v.into_pyobject(py)?])?, None)? + .into_any(), + Value::BigInt(ref v) => PyType::new::(py) + .call(PyTuple::new(py, &[v.into_pyobject(py)?])?, None)? + .into_any(), + Value::Boolean(b) => b.into_pyobject(py)?.to_owned().into_any(), }; - Ok(v) + Ok(v.unbind().into_any()) } diff --git a/edb/graphql-rewrite/src/py_token.rs b/edb/graphql-rewrite/src/py_token.rs index 9143709fa64..edd738cfc4f 100644 --- a/edb/graphql-rewrite/src/py_token.rs +++ b/edb/graphql-rewrite/src/py_token.rs @@ -2,7 +2,7 @@ use edb_graphql_parser::common::{unquote_block_string, unquote_string}; use edb_graphql_parser::position::Pos; use edb_graphql_parser::tokenizer::Token; use pyo3::prelude::*; -use pyo3::types::{PyList, PyTuple}; +use pyo3::types::{PyList, PyString, PyTuple}; use std::borrow::Cow; use crate::py_exception::LexingError; @@ -73,42 +73,42 @@ impl PyToken { } } -pub fn convert_tokens( - py: Python, +pub fn convert_tokens<'py>( + py: Python<'py>, tokens: &[PyToken], end_pos: &Pos, kinds: PyObject, -) -> PyResult { +) -> PyResult> { use PyTokenKind as K; let sof = kinds.getattr(py, "SOF")?; let eof = kinds.getattr(py, "EOF")?; let bang = kinds.getattr(py, "BANG")?; - let bang_v: PyObject = "!".into_py(py); + let bang_v = "!".into_pyobject(py)?; let dollar = kinds.getattr(py, "DOLLAR")?; - let dollar_v: PyObject = "$".into_py(py); + let dollar_v = "$".into_pyobject(py)?; let paren_l = kinds.getattr(py, "PAREN_L")?; - let paren_l_v: PyObject = "(".into_py(py); + let paren_l_v = "(".into_pyobject(py)?; let paren_r = kinds.getattr(py, "PAREN_R")?; - let paren_r_v: PyObject = ")".into_py(py); + let paren_r_v = ")".into_pyobject(py)?; let spread = kinds.getattr(py, "SPREAD")?; - let spread_v: PyObject = "...".into_py(py); + let spread_v = "...".into_pyobject(py)?; let colon = kinds.getattr(py, "COLON")?; - let colon_v: PyObject = ":".into_py(py); + let colon_v = ":".into_pyobject(py)?; let equals = kinds.getattr(py, "EQUALS")?; - let equals_v: PyObject = "=".into_py(py); + let equals_v = "=".into_pyobject(py)?; let at = kinds.getattr(py, "AT")?; - let at_v: PyObject = "@".into_py(py); + let at_v = "@".into_pyobject(py)?; let bracket_l = kinds.getattr(py, "BRACKET_L")?; - let bracket_l_v: PyObject = "[".into_py(py); + let bracket_l_v = "[".into_pyobject(py)?; let bracket_r = kinds.getattr(py, "BRACKET_R")?; - let bracket_r_v: PyObject = "]".into_py(py); + let bracket_r_v = "]".into_pyobject(py)?; let brace_l = kinds.getattr(py, "BRACE_L")?; - let brace_l_v: PyObject = "{".into_py(py); + let brace_l_v = "{".into_pyobject(py)?; let pipe = kinds.getattr(py, "PIPE")?; - let pipe_v: PyObject = "|".into_py(py); + let pipe_v = "|".into_pyobject(py)?; let brace_r = kinds.getattr(py, "BRACE_R")?; - let brace_r_v: PyObject = "}".into_py(py); + let brace_r_v = "}".into_pyobject(py)?; let name = kinds.getattr(py, "NAME")?; let int = kinds.getattr(py, "INT")?; let float = kinds.getattr(py, "FLOAT")?; @@ -117,77 +117,76 @@ pub fn convert_tokens( let mut elems: Vec = Vec::with_capacity(tokens.len()); + let zero = 0u32.into_pyobject(py).unwrap(); let start_of_file = [ sof.clone_ref(py), - 0u32.into_py(py), - 0u32.into_py(py), - 0u32.into_py(py), - 0u32.into_py(py), + zero.clone().into(), + zero.clone().into(), + zero.clone().into(), + zero.clone().into(), py.None(), ]; - elems.push(PyTuple::new_bound(py, &start_of_file).into()); + elems.push(PyTuple::new(py, &start_of_file)?.into()); for token in tokens { let (kind, value) = match token.kind { K::Sof => (sof.clone_ref(py), py.None()), K::Eof => (eof.clone_ref(py), py.None()), - K::Bang => (bang.clone_ref(py), bang_v.clone_ref(py)), - K::Dollar => (dollar.clone_ref(py), dollar_v.clone_ref(py)), - K::ParenL => (paren_l.clone_ref(py), paren_l_v.clone_ref(py)), - K::ParenR => (paren_r.clone_ref(py), paren_r_v.clone_ref(py)), - K::Spread => (spread.clone_ref(py), spread_v.clone_ref(py)), - K::Colon => (colon.clone_ref(py), colon_v.clone_ref(py)), - K::Equals => (equals.clone_ref(py), equals_v.clone_ref(py)), - K::At => (at.clone_ref(py), at_v.clone_ref(py)), - K::BracketL => (bracket_l.clone_ref(py), bracket_l_v.clone_ref(py)), - K::BracketR => (bracket_r.clone_ref(py), bracket_r_v.clone_ref(py)), - K::BraceL => (brace_l.clone_ref(py), brace_l_v.clone_ref(py)), - K::Pipe => (pipe.clone_ref(py), pipe_v.clone_ref(py)), - K::BraceR => (brace_r.clone_ref(py), brace_r_v.clone_ref(py)), - K::Name => (name.clone_ref(py), token.value.clone().into_py(py)), - K::Int => (int.clone_ref(py), token.value.clone().into_py(py)), - K::Float => (float.clone_ref(py), token.value.clone().into_py(py)), + K::Bang => (bang.clone_ref(py), bang_v.to_owned().into()), + K::Dollar => (dollar.clone_ref(py), dollar_v.to_owned().into()), + K::ParenL => (paren_l.clone_ref(py), paren_l_v.to_owned().into()), + K::ParenR => (paren_r.clone_ref(py), paren_r_v.to_owned().into()), + K::Spread => (spread.clone_ref(py), spread_v.to_owned().into()), + K::Colon => (colon.clone_ref(py), colon_v.to_owned().into()), + K::Equals => (equals.clone_ref(py), equals_v.to_owned().into()), + K::At => (at.clone_ref(py), at_v.to_owned().into()), + K::BracketL => (bracket_l.clone_ref(py), bracket_l_v.to_owned().into()), + K::BracketR => (bracket_r.clone_ref(py), bracket_r_v.to_owned().into()), + K::BraceL => (brace_l.clone_ref(py), brace_l_v.to_owned().into()), + K::Pipe => (pipe.clone_ref(py), pipe_v.to_owned().into()), + K::BraceR => (brace_r.clone_ref(py), brace_r_v.to_owned().into()), + K::Name => (name.clone_ref(py), PyString::new(py, &token.value).into()), + K::Int => (int.clone_ref(py), PyString::new(py, &token.value).into()), + K::Float => (float.clone_ref(py), PyString::new(py, &token.value).into()), K::String => { // graphql-core 3 receives unescaped strings from the lexer let v = unquote_string(&token.value) .map_err(|e| LexingError::new_err(e.to_string()))? - .into_py(py); - (string.clone_ref(py), v) + .into_pyobject(py)?; + (string.clone_ref(py), v.to_owned().into()) } K::BlockString => { // graphql-core 3 receives unescaped strings from the lexer let v = unquote_block_string(&token.value) .map_err(|e| LexingError::new_err(e.to_string()))? - .into_py(py); - (block_string.clone_ref(py), v) + .into_pyobject(py)?; + (block_string.clone_ref(py), v.to_owned().into()) } }; - let token_tuple = [ + let token_tuple = ( kind, - token.position.map(|x| x.character).into_py(py), + token.position.map(|x| x.character), token .position - .map(|x| x.character + token.value.chars().count()) - .into_py(py), - token.position.map(|x| x.line).into_py(py), - token.position.map(|x| x.column).into_py(py), + .map(|x| x.character + token.value.chars().count()), + token.position.map(|x| x.line), + token.position.map(|x| x.column), value, - ]; - elems.push(PyTuple::new_bound(py, &token_tuple).into()); + ) + .into_pyobject(py)?; + elems.push(token_tuple.into()); } elems.push( - PyTuple::new_bound( - py, - &[ - eof.clone_ref(py), - end_pos.character.into_py(py), - end_pos.line.into_py(py), - end_pos.column.into_py(py), - end_pos.character.into_py(py), - py.None(), - ], + ( + eof, + end_pos.character, + end_pos.line, + end_pos.column, + end_pos.character, + py.None(), ) - .into(), + .into_pyobject(py)? + .into(), ); - Ok(PyList::new_bound(py, &elems[..]).into()) + PyList::new(py, elems) } diff --git a/edb/server/conn_pool/src/python.rs b/edb/server/conn_pool/src/python.rs index 5f420f1d862..543c82e4219 100644 --- a/edb/server/conn_pool/src/python.rs +++ b/edb/server/conn_pool/src/python.rs @@ -14,6 +14,7 @@ use std::{ os::fd::IntoRawFd, pin::Pin, rc::Rc, + sync::Mutex, thread, time::{Duration, Instant}, }; @@ -37,21 +38,22 @@ enum RustToPythonMessage { Metrics(Vec), } -impl ToPyObject for RustToPythonMessage { - fn to_object(&self, py: Python<'_>) -> PyObject { +impl RustToPythonMessage { + fn to_object(&self, py: Python<'_>) -> PyResult { use RustToPythonMessage::*; match self { - Acquired(a, b) => (0, a, b.0).to_object(py), - PerformConnect(conn, s) => (1, conn.0, s).to_object(py), - PerformDisconnect(conn) => (2, conn.0).to_object(py), - PerformReconnect(conn, s) => (3, conn.0, s).to_object(py), - Pruned(conn) => (4, conn).to_object(py), - Failed(conn, error) => (5, conn, error.0).to_object(py), + Acquired(a, b) => (0, a, b.0).into_pyobject(py), + PerformConnect(conn, s) => (1, conn.0, s).into_pyobject(py), + PerformDisconnect(conn) => (2, conn.0).into_pyobject(py), + PerformReconnect(conn, s) => (3, conn.0, s).into_pyobject(py), + Pruned(conn) => (4, conn).into_pyobject(py), + Failed(conn, error) => (5, conn, error.0).into_pyobject(py), Metrics(metrics) => { // This is not really fast but it should not be happening very often - (6, PyByteArray::new_bound(py, &metrics)).to_object(py) + (6, PyByteArray::new(py, &metrics)).into_pyobject(py) } } + .map(|e| e.unbind().into_any()) } } @@ -175,7 +177,7 @@ impl Connector for Rc { #[pyclass] struct ConnPool { python_to_rust: tokio::sync::mpsc::UnboundedSender, - rust_to_python: std::sync::mpsc::Receiver, + rust_to_python: Mutex>, notify_fd: u64, } @@ -328,7 +330,7 @@ impl ConnPool { let notify_fd = rxfd.recv().unwrap(); ConnPool { python_to_rust: txpr, - rust_to_python: rxrp, + rust_to_python: Mutex::new(rxrp), notify_fd, } } @@ -374,16 +376,26 @@ impl ConnPool { .map_err(|_| internal_error("In shutdown")) } - fn _read(&self, py: Python<'_>) -> Py { - let Ok(msg) = self.rust_to_python.recv() else { - return py.None(); + fn _read(&self, py: Python<'_>) -> PyResult> { + let Ok(msg) = self + .rust_to_python + .try_lock() + .expect("Unsafe thread access") + .try_recv() + else { + return Ok(py.None()); }; msg.to_object(py) } - fn _try_read(&self, py: Python<'_>) -> Py { - let Ok(msg) = self.rust_to_python.try_recv() else { - return py.None(); + fn _try_read(&self, py: Python<'_>) -> PyResult> { + let Ok(msg) = self + .rust_to_python + .try_lock() + .expect("Unsafe thread access") + .try_recv() + else { + return Ok(py.None()); }; msg.to_object(py) } @@ -406,13 +418,12 @@ struct LoggingGuard { impl LoggingGuard { #[new] fn init_logging(py: Python) -> PyResult { - let logging = py.import_bound("logging")?; + let logging = py.import("logging")?; let logger = logging.getattr("getLogger")?.call(("edb.server",), None)?; let level = logger .getattr("getEffectiveLevel")? .call((), None)? .extract::()?; - let logger = logger.to_object(py); struct PythonSubscriber { logger: Py, @@ -475,7 +486,9 @@ impl LoggingGuard { tracing_subscriber::filter::LevelFilter::OFF }; - let subscriber = PythonSubscriber { logger }; + let subscriber = PythonSubscriber { + logger: logger.into(), + }; let guard = tracing_subscriber::registry() .with(level) .with(subscriber) @@ -490,7 +503,7 @@ impl LoggingGuard { fn _conn_pool(py: Python, m: &Bound) -> PyResult<()> { m.add_class::()?; m.add_class::()?; - m.add("InternalError", py.get_type_bound::())?; + m.add("InternalError", py.get_type::())?; // Add each metric variant as a constant for variant in MetricVariant::iter() { diff --git a/edb/server/http/src/python.rs b/edb/server/http/src/python.rs index 1aa2ae5aa0b..217f5bcfb81 100644 --- a/edb/server/http/src/python.rs +++ b/edb/server/http/src/python.rs @@ -35,25 +35,22 @@ enum RustToPythonMessage { SSEEnd(PythonConnId), Error(PythonConnId, String), } - -impl ToPyObject for RustToPythonMessage { - fn to_object(&self, py: Python<'_>) -> PyObject { +impl RustToPythonMessage { + fn to_object(&self, py: Python<'_>) -> PyResult { use RustToPythonMessage::*; trace!("Read: {self:?}"); match self { - Error(conn, error) => (0, *conn, error).to_object(py), - Response(conn, (status, body, headers)) => ( - 1, - conn, - (*status, PyByteArray::new_bound(py, &body), headers), - ) - .to_object(py), - SSEStart(conn, (status, headers)) => (2, conn, (status, headers)).to_object(py), + Error(conn, error) => (0, conn, error).into_pyobject(py), + Response(conn, (status, body, headers)) => { + (1, conn, (status, PyByteArray::new(py, body), headers)).into_pyobject(py) + } + SSEStart(conn, (status, headers)) => (2, conn, (status, headers)).into_pyobject(py), SSEEvent(conn, message) => { - (3, conn, (&message.id, &message.data, &message.event)).to_object(py) + (3, conn, (&message.id, &message.data, &message.event)).into_pyobject(py) } - SSEEnd(conn) => (4, conn, ()).to_object(py), + SSEEnd(conn) => (4, conn).into_pyobject(py), } + .map(|e| e.unbind().into_any()) } } @@ -104,7 +101,7 @@ impl RpcPipe { #[pyclass] struct Http { python_to_rust: tokio::sync::mpsc::UnboundedSender, - rust_to_python: std::sync::mpsc::Receiver, + rust_to_python: Mutex>, notify_fd: u64, } @@ -588,7 +585,7 @@ impl Http { let notify_fd = rxfd.recv().unwrap(); Http { python_to_rust: txpr, - rust_to_python: rxrp, + rust_to_python: Mutex::new(rxrp), notify_fd, } } @@ -634,16 +631,26 @@ impl Http { self.send(PythonToRustMessage::UpdateLimit(limit)) } - fn _read(&self, py: Python<'_>) -> Py { - let Ok(msg) = self.rust_to_python.recv() else { - return py.None(); + fn _read(&self, py: Python<'_>) -> PyResult { + let Ok(msg) = self + .rust_to_python + .try_lock() + .expect("Unsafe thread access") + .recv() + else { + return Ok(py.None()); }; msg.to_object(py) } - fn _try_read(&self, py: Python<'_>) -> Py { - let Ok(msg) = self.rust_to_python.try_recv() else { - return py.None(); + fn _try_read(&self, py: Python<'_>) -> PyResult { + let Ok(msg) = self + .rust_to_python + .try_lock() + .expect("Unsafe thread access") + .try_recv() + else { + return Ok(py.None()); }; msg.to_object(py) } @@ -659,7 +666,7 @@ impl Http { #[pymodule] fn _http(py: Python, m: &Bound) -> PyResult<()> { m.add_class::()?; - m.add("InternalError", py.get_type_bound::())?; + m.add("InternalError", py.get_type::())?; Ok(()) } diff --git a/edb/server/pgrust/src/protocol/gen.rs b/edb/server/pgrust/src/protocol/gen.rs index dd6cb83875e..85b88b78226 100644 --- a/edb/server/pgrust/src/protocol/gen.rs +++ b/edb/server/pgrust/src/protocol/gen.rs @@ -313,7 +313,7 @@ macro_rules! protocol_builder { let Ok(val) = $crate::protocol::FieldAccess::<$type>::extract(buf.split_at(offset).1) else { return false; }; - if val != $value as usize as _ { return false; } + if val as usize != $value as usize { return false; } )? offset += std::mem::size_of::<$type>(); )* diff --git a/edb/server/pgrust/src/python.rs b/edb/server/pgrust/src/python.rs index 076f3d45959..2931a212b23 100644 --- a/edb/server/pgrust/src/python.rs +++ b/edb/server/pgrust/src/python.rs @@ -18,11 +18,11 @@ use pyo3::{ exceptions::{PyException, PyRuntimeError}, prelude::*, pymodule, - types::{PyAnyMethods, PyByteArray, PyBytes, PyMemoryView, PyModule, PyModuleMethods, PyNone}, + types::{PyAnyMethods, PyByteArray, PyBytes, PyMemoryView, PyModule, PyModuleMethods}, Bound, PyAny, PyResult, Python, }; use std::collections::HashMap; -use std::path::Path; +use std::{borrow::Cow, path::Path}; #[derive(Clone, Copy, PartialEq, Eq)] #[pyclass(eq, eq_int)] @@ -116,7 +116,7 @@ impl PyConnectionParams { ResolvedTarget::SocketAddr(addr) => { resolved_hosts.push(( if addr.ip().is_ipv4() { "v4" } else { "v6" }, - addr.ip().to_string().into_py(py), + addr.ip().to_string().into_pyobject(py)?.into(), hostname.clone(), addr.port(), )); @@ -126,7 +126,7 @@ impl PyConnectionParams { if let Some(path) = path.as_pathname() { resolved_hosts.push(( "unix", - path.to_string_lossy().into_py(py), + path.to_string_lossy().into_pyobject(py)?.into(), hostname.clone(), port, )); @@ -141,7 +141,7 @@ impl PyConnectionParams { name.insert(0, 0); resolved_hosts.push(( "unix", - PyBytes::new_bound(py, &name).as_any().clone().unbind(), + PyBytes::new(py, &name).as_any().clone().unbind(), hostname.clone(), port, )); @@ -200,7 +200,7 @@ impl PyConnectionParams { } pub fn resolve(&self, py: Python, username: String, home_dir: String) -> PyResult { - let os = py.import_bound("os")?; + let os = py.import("os")?; let environ = os.getattr("environ")?; let mut params = self.inner.clone(); @@ -215,7 +215,7 @@ impl PyConnectionParams { ¶ms.database, ¶ms.user, )? { - let warnings = py.import_bound("warnings")?; + let warnings = py.import("warnings")?; warnings.call_method1("warn", (warning.to_string(),))?; } @@ -249,8 +249,8 @@ impl PyConnectionParams { repr } - pub fn __getitem__(&self, py: Python, name: &str) -> Py { - self.inner.get_by_name(name).to_object(py) + pub fn __getitem__(&self, name: &str) -> Option> { + self.inner.get_by_name(name) } pub fn __setitem__(&mut self, name: &str, value: &str) -> PyResult<()> { @@ -286,7 +286,7 @@ impl PyConnectionState { username: String, home_dir: String, ) -> PyResult { - let os = py.import_bound("os")?; + let os = py.import("os")?; let environ = os.getattr("environ")?; let mut params = dsn.inner.clone(); @@ -301,7 +301,7 @@ impl PyConnectionState { ¶ms.database, ¶ms.user, )? { - let warnings = py.import_bound("warnings")?; + let warnings = py.import("warnings")?; warnings.call_method1("warn", (warning.to_string(),))?; } @@ -325,15 +325,15 @@ impl PyConnectionState { inner: ConnectionState::new(credentials, ssl_mode), parsed_dsn: Py::new(py, PyConnectionParams { inner: params })?, update: PyConnectionStateUpdate { - py_update: PyNone::get_bound(py).to_object(py), + py_update: py.None(), }, message_buffer: Default::default(), }) } #[setter] - fn update(&mut self, py: Python, update: &Bound) { - self.update.py_update = update.to_object(py); + fn update(&mut self, update: &Bound) { + self.update.py_update = update.clone().unbind(); } fn is_ready(&self) -> bool { @@ -351,7 +351,7 @@ impl PyConnectionState { } fn drive_message(&mut self, py: Python, data: &Bound) -> PyResult<()> { - let buffer = PyBuffer::::get_bound(data)?; + let buffer = PyBuffer::::get(data)?; if self.inner.read_ssl_response() { // SSL responses are always one character let response = [buffer.as_slice(py).unwrap().get(0).unwrap().get()]; @@ -408,7 +408,7 @@ impl ConnectionStateSend for PyConnectionStateUpdate { message: crate::protocol::definition::InitialBuilder, ) -> Result<(), std::io::Error> { Python::with_gil(|py| { - let bytes = PyByteArray::new_bound(py, &message.to_vec()); + let bytes = PyByteArray::new(py, &message.to_vec()); if let Err(e) = self.py_update.call_method1(py, "send", (bytes,)) { eprintln!("Error in send_initial: {:?}", e); e.print(py); @@ -422,7 +422,7 @@ impl ConnectionStateSend for PyConnectionStateUpdate { message: crate::protocol::definition::FrontendBuilder, ) -> Result<(), std::io::Error> { Python::with_gil(|py| { - let bytes = PyBytes::new_bound(py, &message.to_vec()); + let bytes = PyBytes::new(py, &message.to_vec()); if let Err(e) = self.py_update.call_method1(py, "send", (bytes,)) { eprintln!("Error in send: {:?}", e); e.print(py);