pynumaflow-lite: Make mapper APIs more Pythonic. - #374
Conversation
- Removes the redundant keys parameter from handler method. - Installs shutdown handler by default, with an option to override Signed-off-by: Sreekanth <prsreekanth920@gmail.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #374 +/- ##
=======================================
Coverage 92.99% 92.99%
=======================================
Files 68 68
Lines 3656 3656
Branches 236 236
=======================================
Hits 3400 3400
Misses 190 190
Partials 66 66 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Signed-off-by: Sreekanth <prsreekanth920@gmail.com>
Signed-off-by: Sreekanth <prsreekanth920@gmail.com>
| fn fail(&self, error: PyErr) -> Vec<map::Message> { | ||
| // Only the first error is reported; later requests may still be in flight | ||
| // while shutdown is underway, and their failures would be duplicates. | ||
| let mut error_slot = self.error_slot.lock().unwrap(); | ||
| if error_slot.is_none() { | ||
| Python::attach(|py| error.print(py)); | ||
| *error_slot = Some(error); | ||
| } | ||
| drop(error_slot); | ||
|
|
||
| if let Some(tx) = self.shutdown_tx.lock().unwrap().take() { | ||
| let _ = tx.send(()); | ||
| } | ||
|
|
||
| Vec::new() | ||
| } |
There was a problem hiding this comment.
how does this fail work? can you please explain.
There was a problem hiding this comment.
Update this part. I was returning empty vec on failure since the mapper Rust trait needed a Vec to returned. Then convert it to exception on Python side. But this caused issues as only panic is propagated to numa sidecar for displaying in the UI.
This code is updated to use exception groups, updated the PR description with sample output.
Signed-off-by: Sreekanth <prsreekanth920@gmail.com>
vigith
left a comment
There was a problem hiding this comment.
actually how does other popular pyo3 projects do it?
Signed-off-by: Vigith Maurice <vigith@gmail.com>
Syntax change
current:
New:
The
keysparameter is removed, as there is alreadydatum.keys.Our list implementation (
mapper.Messages) is removed, and uses builtinlist.Uncaught exceptions will be logged with
ExceptionGroupinstead of logging the first error.Similar changes will be made to other map variants in a follow up PR