feat(indy-vdr): add indy-vdr package and indy vdr pool #1160
Conversation
Signed-off-by: vickysomtee <[email protected]>
Signed-off-by: vickysomtee <[email protected]>
Signed-off-by: vickysomtee <[email protected]>
Signed-off-by: vickysomtee <[email protected]>
Signed-off-by: vickysomtee <[email protected]>
There was a problem hiding this comment.
Great work @Vickysomtee! Left some comments mostly on things that can be done a bit differently in the indy vdr library compared to the indy sdk
Signed-off-by: vickysomtee <[email protected]>
Work Funded by the Government of Ontario Signed-off-by: vickysomtee <[email protected]>
Signed-off-by: Timo Glastra <[email protected]>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Codecov Report
@@ Coverage Diff @@
## main #1160 +/- ##
===========================================
- Coverage 87.66% 38.74% -48.92%
===========================================
Files 739 613 -126
Lines 17619 14773 -2846
Branches 2992 2520 -472
===========================================
- Hits 15445 5724 -9721
- Misses 2167 9049 +6882
+ Partials 7 0 -7
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
Work Funded by the Government of Ontario Signed-off-by: vickysomtee <[email protected]>
Work Funded by the Government of Ontario Signed-off-by: vickysomtee <[email protected]>
Signed-off-by: Karim Stekelenburg <[email protected]>
Signed-off-by: Karim Stekelenburg <[email protected]>
Signed-off-by: vickysomtee <[email protected]>
Work funded by the Ontario Government Signed-off-by: vickysomtee <[email protected]>
Work funded by the Ontario Government Signed-off-by: vickysomtee <[email protected]>
Signed-off-by: vickysomtee <[email protected]>
Signed-off-by: vickysomtee <[email protected]>
TimoGlastra
changed the title
There was a problem hiding this comment.
Looks good! I've left a few comments and questions about the initialization.
Probably as part of another PR (when adding consumers to Indy VDR Pool like the resolvers and registry), but should we create it as a module that registers services and also initializes the binding library within its register method?
| @@ -0,0 +1,5 @@ | |||
| try { | |||
| require('indy-vdr-test-nodejs') | |||
There was a problem hiding this comment.
How will this work when we want to use this package in React Native? Should we add the require('indy-vdr-test-react-native') to the catch block?
There was a problem hiding this comment.
I was thinking we could either do it with the catch as you say, or we detect the platform. Or would you prefer the manual approach? (so you provide it to the module). That's also fine for me.
| } | ||
|
|
||
| // FIXME: this method doesn't work?? | ||
| // this.pool.close() |
There was a problem hiding this comment.
Is it throwing an error or something like that?
There was a problem hiding this comment.
Yes. it hangs indefinitely. should create an issue for it in the indy-vdr repo. @blu3beri do you still know the details?
There was a problem hiding this comment.
Ah yeah, apparently, almost a year ago now, I create an issue for this.
hyperledger/indy-vdr#92
There was a problem hiding this comment.
I can investigate this further if it's required. The reasoning (no write-access to lock) might be completely incorrect.
Signed-off-by: vickysomtee <[email protected]>
Work funded by the Ontario Government Signed-off-by: vickysomtee <[email protected]>
Work funded by the Ontario Government Signed-off-by: vickysomtee <[email protected]>
Work funded by the Ontario Government Signed-off-by: vickysomtee <[email protected]>
There was a problem hiding this comment.
Looks like these are mostly style changes and they look good to me. I'm assuming the README will be done in a follow-up PR, so I'll approve. If not, this needs to be added before merging.
In order to do so, just have a look at the READMEs of other packages inside this repository. They'll give you an idea of what is required.
| describe('Schemas & credential Definition', () => { | ||
| test('can write a schema using the pool', async () => { | ||
| const pool = indyVdrPoolService.getPoolForNamespace('pool:localtest') | ||
|
|
||
| const dynamicVersion = `1.${Math.random() * 100}` // TODO Remove this before pushing | ||
| const dynamicVersion = `1.${Math.random() * 100}` |
There was a problem hiding this comment.
@TimoGlastra this was originally added by me. I assumed we could remove this, as the CI uses a clean ledger for every run. Is my assumption correct?
There was a problem hiding this comment.
Yeah we can. But then subsequent test runs (local) will fail I think? So I thought there's not harm in keeping it in.
work funded by the Ontario Government Signed-off-by: vickysomtee <[email protected]>
|
I think it would be nice to use @hyperledger/indy-vdr-* packages now that they are going to be released in NPM after the v0.4.0-dev.2 release (https://github.com/hyperledger/indy-vdr/releases/tag/v0.4.0-dev.2) |
work funded by the Ontario Government Signed-off-by: Victor Anene <[email protected]>
It doesn't quite work yet, so I'm going to merge this once CI passes and once the build is working we can make a follow up PR. |
work funded by the Ontario Government Signed-off-by: Victor Anene <[email protected]>
This PR implements the new IndyVdr Pool Package
Work funded by the government of Ontario