feat: Add lens-wasm instance repository#110
Conversation
Testify also bumped out of laziness
Only changes are the removal of Defra dependencies (done by copying the required parts of their interfaces/aliases to Lens)
Lens prefix is not helpful in the lens repo, and Document is prefered of Doc now that the type is public
So that it is appropriate for the Lens repo. No code has changed, only names of stuff.
AndrewSisley
requested review from
ChrisBQu,
fredcarle,
islamaliev,
nasdf and
shahzadlone
fredcarle
left a comment
fredcarle
left a comment
There was a problem hiding this comment.
I like the new interfaces. Just one non-blocking suggestion.
|
|
||
| func (r *explicitTxnRepository) Init(txnSource TxnSource) {} | ||
|
|
||
| func (r *explicitTxnRepository) WithTxn(txn Txn) Repository { |
There was a problem hiding this comment.
suggestion: WithTxn on implicitTxnRepository was removed at one point. If you want to eventually make the explicit transaction work, this will be needed.
There was a problem hiding this comment.
I didn't think it ever was on implicitTxnRepository - I didnt check though. I assumed the implicitTxnDb just had a reference to concrete type instead of the interface.
Going to leave this as-is for now, as it's omission makes the explicitTxnRepository dead code, and I don't really want to re-expose it to users before it is tested again.
There was a problem hiding this comment.
I can confirm that it was :)
I'm ok with that. We'll just have to figure out a way to reintroduce explicit transactions at some point if we care to have lens respect them.
## Relevant issue(s) Resolves #3869 ## Description Replaces the lens registry with the new Lens repository in the Lens repo - introduced in this PR sourcenetwork/lens#110 Also updates Lens to v0.8.1 - which includes the repo url changes.
## Relevant issue(s) Resolves #3869 ## Description Replaces the lens registry with the new Lens repository in the Lens repo - introduced in this PR sourcenetwork/lens#110 Also updates Lens to v0.8.1 - which includes the repo url changes.
2 participants
Relevant issue(s)
Resolves #109
Description
Adds lens-wasm instance repository, to manage pooled instances of wasm modules.
Pulled from the Defra repo with naming/documentation changes to make it use-case agnostic. Nothing has changed behaviour wise from there.
For now this is not tested by the Lens repo, I have tested it via the Defra repo. Lense will gain proper testing in this ticket #108 - this will probably be done after introducing on-disk storage, and an http client - so that the current binary/language agnostic tests can be safely ported to the testo framework.
The explicit transaction stuff was unused in Defra, but is assumed to still be working. It is not currently tested, but that will change once we fix either the Defra side, or add tests hosted in the Lens repo.
The corresponding Defra PR is here sourcenetwork/defradb#3874.