search

Glimpse / Glimpse.Prototype

Glimpse v2 prototype
MIT License
185 stars 42 forks source link

Integrate In-memory persistence store #16

Closed nikmd23 closed 9 years ago

nikmd23 commented 9 years ago

@nikmd23 has already built a working spike of the in-memory persistence store.

This work is to integrate it back into the Glimpse repo.

avanderhoorn commented 9 years ago

Just so we know this change will break the clients.

nikmd23 commented 9 years ago

Yes. I assumed the IMessage changes would be breaking.

I'll also add an Ordinal to the interface while I do this, which should complete interface changes for the foreseeable future.

avanderhoorn commented 9 years ago

Yep, I can't think of anything else for the moment

nikmd23 commented 9 years ago

I'm currently stalled on this due to the Beta7-12302 bug. Will have to resume once we have a working build again. :disappointed:

avanderhoorn commented 9 years ago

As of 12321 this should unblocked

nikmd23 commented 9 years ago

There's been a proposal to do away with IMessage in IStorage and related interfaces, and instead just use JSON strings.

avanderhoorn commented 9 years ago

Great question. It makes complete sense to me, that if we can avoid rehydrating the message out of the store only to turn it to JSON again it would be a big win. Thinking through it, I think that you should have everything you need about a message from the index which means you shouldn't have to look inside the message itself. Are there any possible use cases where we would need IMessage going out of the store to the client that you can think of? Assuming not, I think we should do it sooner rather than later. I t would be more difficult to change once people start creating stores. That said, maybe it would make a good "jump-in" for the next little while whilst we get some of these bigger tasks done.

nikmd23 commented 9 years ago

The last thing that needs to be done on this branch now is the switch over to making Message.Payload a JSON string of the full (?) message.

nikmd23 commented 9 years ago

With the exception of tests, I think this now includes all the changes we've talked about. @avanderhoorn I'll let you inspect and :shipit:

nikmd23 commented 9 years ago

We should merge this in soon, it's blocking #25. Of course, it has to be rebased again, which I'll do now.

avanderhoorn commented 9 years ago

From what I can tell it looks good. There is a lot here, so if you want to pull it in and run it through with me at some point that would be good.

nikmd23 commented 9 years ago

I'm happy to review this whenever is good for you.