-
-
Notifications
You must be signed in to change notification settings - Fork 690
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merge the handling of on_result and async result handling #2264
Conversation
core/tests/test_handlers.py
Outdated
# The answer was still passed to the callback | ||
on_result.assert_called_once_with(42) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The original Android code didn't do this, on the basis that since we're exposing a cancellation mechanism, it would make sense for that to cancel the on_result
call as well, because the app probably isn't able to accept it after cancellation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense; I've updated the code to do this.
I wonder if rather than adding more def f(self):
self.webview.evaluate_javascript(js, on_result=self.on_result)
def on_result(self, result, exception=None):
if exception:
...
else:
... If def f(self):
asyncio.create_task(self.on_result(self.webview.evaluate_javascript(js)))
async def on_result(self, aresult):
try:
result = await aresult
...
except Exception:
... But in almost all cases, Apart from being more readable, the advantage of not calling |
I'm not sure I follow your argument here. The If we were going to go down this path, I'd be more inclined to suggest that we drop |
Following in-person discussion with @mhsmith: In an ideal world,
The good news is that the usage of async that is needed in this case is relatively limited - "just add With that in mind, we're going to :
We won't finalise the deprecation of |
Another discovery in the process of developing a Camera API.
Toga has an internal AsyncResult handler that is used for methods that need to operate in an async context, but need to retain synchronous calling semantics. Whenever an AsyncResult is defined, there is an on_result lurking nearby; however, everywhere that this is done, the "handle async result, then handle on_result" logic is duplicated.
This PR merges the on_result handler into the AsyncResult, so that any time a result is reported, it is passed to both the synchronous and asynchronous contexts.
This also:
on_result
entirely; users will be encouraged to useawait
.PR Checklist: