search

replikativ / konserve

A clojuresque key-value/document store protocol with core.async.
Eclipse Public License 1.0
299 stars 25 forks source link

54 add synchronous protocols #57

Closed whilo closed 2 years ago

whilo commented 3 years ago

Implement synchronous IO.

whilo commented 3 years ago

Yes, I will remove it and squash.

whilo commented 3 years ago

Should the metadata.org-file be in here and when yes, should it be more polished then? It is not clear to me what it is for.

I would really like to see some kind of ADR in konserve to understand the recent changes and especially why this actual change is necessary. It is good to communicate especially breaking changes in a persistent way that can be comprehended later as well.

I removed the metadata file and will try to document more of the design decisions, although they have accumulated over the years and would probably be a pretty big ADR. Can you maybe elaborate a bit what is not clear from the current backend documentation and from Alex' talk?

whilo commented 3 years ago

I have now factored all the serialization logic from the invocation of low-level IO and all that should be needed from now on is just to implement the low-level protocols and use the new default implementation for the high-level protocols.

whilo commented 3 years ago

I would consider this mostly done, modulo renamings and adding documentation. But I am happy to collect one further round of feedback before merging.

whilo commented 3 years ago

Also it would be good to add compliance tests for the low-level protocols now, I think running the high-level ones is too indirect, but I don't have time in the next week to do so and I think it can be done later. It would also not be bad if somebody without experience of the protocols would do it and I would peer-review, so we see what needs to be documented or potentially changed.

whilo commented 2 years ago

I have now also updated the backend.org file and tried to provide as much context as possible for backend implementers, but I am sure it is not yet optimal. Please take a look and make sure that we sort things out now and konserve is as consistent and accessible as possible.

jsmassa commented 2 years ago

Are the protocols meant to be still called PEDNAsyncKeyValueStore and PBinaryAsyncKeyValueStore for backwards compatibility?

whilo commented 2 years ago

We can rename them and remove the Async if this is what you are suggesting. Technically speaking I still expect stores to implement asynchronous execution if possible, but they don't have to.