fix(python_adapter): more __yield_messages fixes#202
Conversation
fpacifici
left a comment
fpacifici
left a comment
There was a problem hiding this comment.
Please ensure this still works.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
bmckerry
force-pushed
the
ben/fix-rust-step-code
branch
from
f11b4e3 to
5372e47
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
fpacifici
left a comment
fpacifici
left a comment
There was a problem hiding this comment.
Please see the issue with From
| }) | ||
| .unwrap() | ||
| } | ||
| } |
There was a problem hiding this comment.
Cursor is right. From cannot fail. You should use TryFrom for type conversions that can fail
https://doc.rust-lang.org/std/convert/trait.TryFrom.html
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
bmckerry
changed the title
| } | ||
|
|
||
| #[test] | ||
| fn test_submit_watermark() { |
There was a problem hiding this comment.
This test was almost entirely redundant and only passed because make_test_watermark had the wrong route. I moved submitting a watermark into the above test_submit_with_matching_route test (the diff makes this look weird)
| } | ||
|
|
||
| fn make_test_watermark(py: Python<'_>) -> Message<RoutedValue> { | ||
| fn make_test_watermark() -> Message<RoutedValue> { |
There was a problem hiding this comment.
This was completely broken and only worked because it returned a message with the wrong route.
3 participants
#203 fixed the initial error by returning messages as a Sequence instead of a generator, but this caused another issue where watermarks weren't handled by the python adapter's
handle_py_return_valuefunction.This required implementing
From Py<PyAny> for Watermarkto allow converting the pythonPyWatermarkreturned from thePythonAdapterinto a rustPyWatermark.A rust test for this code path is coming in a future PR.