Re: [MirageOS-devel] Irmin API evolution

[Thomas Gazagnaire <thomas@xxxxxxxxxxxxxx>]

> However, I think we should remove "create" from the store interfaces
> anyway. In Irmin-IndexedDB, I want to pass database connections to the
> internal RW and AO stores, not a config. It looks like I can attach
> arbitrary data to config objects, but doing this loses compile-time
> type checking (it would be possible to call it without providing a
> connection).

I agree. Your examples are convincing, and I think having the high-level and 
low-level stores having a different interface makes sense as it will allow to 
make the best design choices at each layer.

>>> [ And ideally, it would make more sense to me if you only specified
>>> the commit message when making a commit. The rest of the strings just
>>> get thrown away, I think. ]
>> 
>> The initial idea was to use that task to (i) populate an audit log on all 
>> the database operations (including reads) and (ii) attach the debug messages 
>> to the task, instead of throwing them on the error channel. None of these 
>> have been completed yet, but would be nice if they are still possible to do 
>> later.
> 
> These things don't sound very Irmin-specific, or connected to the Git
> log message. Perhaps we could provide a wrapper for logging?
> 
> module AuditedStore (S : BC.STORE) : sig
>  include BC.STORE
>  val create : Logger.t -> S.t -> t
>  ...
> end

What if you want to commit on every read (costly, but it'd be possible to do 
now by passing a flag in the config)?

Also, my initial idea for logging was to pass a store handle (and its task) to 
the Log.debug functions, so that the log line is appended in the task's commit 
message (that's why there is a `Task.add`). By doing so, we could have all the 
debug message for a specific high-level command appears in the commit message. 
Not sure how practical is it, but I though it could be a nice feature.

I'm not sure how you can do that if you remove the task from all the read 
operations.

On the other hand, having immutable, simple read-operations is very useful when 
designing the API. Currently, the HTTP "REST" [1] API doesn't require a client 
task for read operations. Would be good to have a consistent API across all 
backends, so that could be an argument for dropping the task for high-level 
reads.


About the wrapper: it's a good idea, we could have a default "audit" branch to 
store this log.

Thomas

[1] https://github.com/mirage/irmin/issues/282#issuecomment-136806513
_______________________________________________
MirageOS-devel mailing list
MirageOS-devel@xxxxxxxxxxxxxxxxxxxx
http://lists.xenproject.org/cgi-bin/mailman/listinfo/mirageos-devel