-
Notifications
You must be signed in to change notification settings - Fork 286
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(fabric): serialization of ccp and sshconfig #3578
Conversation
f3c02d4
to
a346e83
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've left two open questions, please have a look :)
.../cactus-plugin-ledger-connector-fabric/src/main/typescript/plugin-ledger-connector-fabric.ts
Outdated
Show resolved
Hide resolved
...src/test/typescript/integration/fabric-v2-2-x/run-transaction-endpoint-v1-serialized.test.ts
Outdated
Show resolved
Hide resolved
a346e83
to
4eec642
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with comments
.../cactus-plugin-ledger-connector-fabric/src/main/typescript/plugin-ledger-connector-fabric.ts
Outdated
Show resolved
Hide resolved
.../cactus-plugin-ledger-connector-fabric/src/main/typescript/plugin-ledger-connector-fabric.ts
Outdated
Show resolved
Hide resolved
.../cactus-plugin-ledger-connector-fabric/src/main/typescript/plugin-ledger-connector-fabric.ts
Outdated
Show resolved
Hide resolved
...tor-fabric/src/test/typescript/integration/fabric-v2-2-x/run-transaction-endpoint-v1.test.ts
Outdated
Show resolved
Hide resolved
...tor-fabric/src/test/typescript/integration/fabric-v2-2-x/run-transaction-endpoint-v1.test.ts
Outdated
Show resolved
Hide resolved
4eec642
to
816d14f
Compare
816d14f
to
f6779c6
Compare
.../cactus-plugin-ledger-connector-fabric/src/main/typescript/plugin-ledger-connector-fabric.ts
Outdated
Show resolved
Hide resolved
f6779c6
to
21f4394
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thank you for the updates!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jagpreetsinghsasan We are almost there! Please fix the tests that started failing. I saw this in the logs:
AIL packages/cactus-plugin-ledger-connector-fabric/src/test/typescript/integration/fabric-v2-2-x/run-transaction-endpoint-v1.test.ts (177.176 s, 1102 MB heap size)
● runs tx on a Fabric v2.5.6 ledger › runs tx on a Fabric v2.5.6 ledger
Cannot instantiate Fabric connector without connection profile.
292 | this.connectionProfile = JSON.parse(connectionProfileString);
293 | } else {
> 294 | throw new Error(
| ^
295 | "Cannot instantiate Fabric connector without connection profile.",
296 | );
297 | }
at new PluginLedgerConnectorFabric (packages/cactus-plugin-ledger-connector-fabric/src/main/typescript/plugin-ledger-connector-fabric.ts:294:13)
at Object.<anonymous> (packages/cactus-plugin-ledger-connector-fabric/src/test/typescript/integration/fabric-v2-2-x/run-transaction-endpoint-v1.test.ts:158:20)
21f4394
to
ea2f752
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jagpreetsinghsasan Thank you for the updates, looking good now. Please fix the linter errors and then we are good to go from my side.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jagpreetsinghsasan Please fix the linter errors
362abf8
to
3c61af7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jagpreetsinghsasan LGTM, thank you!
Primary Changes --------------- 1. sshConfig and connectionProfile can now be passed as base64 encoded strings Changes required to incorporate 1) --------------------------------- 2. Added sshConfigBase64Encoded and connectionProfileBase64encoded properties to IPluginLedgerConnectorFabricOptions 3. Added necessary FabricConnector class variables 4. Added a new testcase to demonstrate the same 5. Updated an implementation which fetches sshConfig from class variable Fixes hyperledger-cacti#3577 Signed-off-by: jagpreetsinghsasan <[email protected]>
Commit to be reviewed
feat(fabric): serialization of ccp and sshconfig
Fixes #3577
Pull Request Requirements
upstream/main
branch and squashed into single commit to help maintainers review it more efficient and to avoid spaghetti git commit graphs that obfuscate which commit did exactly what change, when and, why.-s
flag when usinggit commit
command. You may refer to this link for more information.Character Limit
A Must Read for Beginners
For rebasing and squashing, here's a must read guide for beginners.