No frills Result class #612
Conversation
| return self | ||
|
|
||
|
|
||
| class NoResult: |
There was a problem hiding this comment.
Is it supposed to be checked that this is an instance of a NoResult when used? i.e. can it be guaranteed that the read/write functions won't be called on a NoResult
There was a problem hiding this comment.
So I'll have to see what feels right when I start using this everywhere, but the current plan is that that will just error out.
There was a problem hiding this comment.
Error out like it tries to access a function on a class that doesn't have it?
There was a problem hiding this comment.
It may be worthwhile to implement read()/write() just so we can have a more informative error than missing attribute -- although depending on how you use this (in next PR, I believe) that could change. Happy to wait and see.
There was a problem hiding this comment.
Yea if it's OK with y'all, I do agree I'll need to do something special for the situation @joshmeek brought up but I'm not 100% sure what the best way to handle it is yet
Co-Authored-By: cicdw <white.cdw@gmail.com>
| assert r.value == 2 | ||
| assert r.handled is False | ||
|
|
||
| def test_result_doesnt_allow_handled_without_result_handler(self): |
…the-interface [db-abstract] more abstractions off of the interface
Thanks for contributing to Prefect!
Please describe your work and make sure your PR:
CHANGELOG.md(if appropriate)docs/outline.tomlfor API reference docs (if appropriate)What does this PR change?
This PR begins work on PIN 4 (and consequently, changes the status of PIN 4 to "Accepted"). Specifically, this PR introduces a new
Resultclass and a correspondingNoResultclass, along with schemas for both. Additionally, this PR makes a large breaking change that result handler methods have been renamed toread/writefor clarity / less ambiguity.Why is this PR important?
This PR is the first step towards the PIN 4 refactor; this PR introduces the necessary classes, but doesn't use them anywhere yet.