1/3 Implement improved API implementation for database #227
Conversation
rkuris
force-pushed
the
rkuris/dbrev-new-apitite
branch
from
566f1ff
to
6ff89df
Compare
While it would be great to return references, the lifetime issues in shale must be worked out first, so we're punting and returning owned values instead for now.
Proofs need to how to fetch data from a node they are operating on.
The proofs are a little harder due to ownership issues.
Per review
rkuris
force-pushed
the
rkuris/dbrev-new-apitite
branch
from
6ff89df
to
6a30d8c
Compare
rkuris
changed the title
rkuris
changed the title
| Err(e) => Err(api::Error::IO(std::io::Error::new(ErrorKind::Other, e))), | ||
| Ok(obj) => match obj { | ||
| None => Ok(None), | ||
| Some(inner) => Ok(Some(inner.as_ref().to_owned())), |
There was a problem hiding this comment.
nit: Naming is hard.., we discussed to no longer use inner as it didn't bring any clarification?
There was a problem hiding this comment.
I think we need to stop using the inner pattern. Naming something inner when we've simply added a wrapper type is fine IMO
There was a problem hiding this comment.
Yeah. This is one of those sweep tasks we can assign as a good first issue. Anyone want to create one?
There was a problem hiding this comment.
I don't think it's trivial to unravel all the SomeType { inner: SomeInnerType } especially with all the misused trait-bounds
| _key: K, | ||
| ) -> Result<Option<Proof<N>>, api::Error> { | ||
| todo!() | ||
| } |
There was a problem hiding this comment.
If these are todo maybe we can do it in a follow up with real implementations?
There was a problem hiding this comment.
That's why this is 1/3 :)
| .map_err(|e| api::Error::IO(std::io::Error::new(ErrorKind::Other, e))) | ||
| } | ||
|
|
||
| async fn val<K: api::KeyType>(&self, key: K) -> Result<Option<Vec<u8>>, api::Error> { |
There was a problem hiding this comment.
It's infuriating that you had to change this.
When we complete the following, we will be able to put it back:
#290
One step along the way, broken up for easier review. Changes include: