feat(plugin-persistence-fabric): add new fabric persistence plugin #2331

barnapa · 2023-03-20T14:38:38Z

Add a new plugin for storing hyperledger fabric ledger data into a database
Add functional tests for plugin and data access layer operations.
operating with fabric-all-in-one docker composition
Tests assume any postgres database, but for final deployment postgres or supabase is assumed.
Data fed by this plugin can later be visualized by a GUI application or analyzed directly. ( right now the GUI will have switch to operate either with Ethereum ledger or Fabric ledger )

Depends on: #2259
Depends on: #2265

Signed-off-by: Barnaba Pawelczak [email protected]

petermetz

@barnapa Please re-organize the commits a little bit to make it easier to review (right now the diff is huge because of the parent PRs in the stack and the 4 commits you have in the PR all look like they belong to your PR itself (but then when I look at the diff I can tell that the parent PRs diff are also included because it has the cmd-gui-app package in too).

If you reorganize your commits so that:

There's only one commit with the changes that belong to this PR and
The other 2 commits come from the 2 parent PRs
then we can review just your specific commit by filtering the diff down to just that (but right now this is not possible).

If you need any help with making this happen (git mechanics or otherwise) then just let me know or join one of the daily pair programming calls that we have and we can screen share: https://wiki.hyperledger.org/display/cactus/Daily+Pair+Programming+Calls

barnapa · 2023-03-28T20:09:49Z

ok it is re organized

petermetz

ok it is re organized

@barnapa It is re-organized, but all you did from what I can tell is squash everything together instead of separating the parent PR commits from this PR. You have a single commit that has a 115 files in its diff and it has the GUI app code in it too. Please double check #2331 (review)

petermetz · 2023-03-30T21:25:08Z

there are some changes in gui also. (parent GUI covers only ethereum ledger, this gui cover also fabric ledger ) so make another PR just for changes in GUI ?

changes for plugin ethereum persistence are not included.

@barnapa
You could open a separate PR for the GUI changes related to the fabric persistence plugin, yes.
We'll still need to separate out the commits in this PR though so that it is differentiated what goes in with this PR and what goes in with the parent PRs. I don't know how else to explain it other than this plus what I said in #2331 (review) but I could either show you live or the slower and easier option is to just wait until the parent gets merged and then you won't have to do the commit surgery, you can just do a rebase onto upstream main and all will be well.
I'm fine with either one of those options. You go with your preference depending on how urgent the review on this PR is.

barnapa · 2023-04-03T08:02:09Z

I removed gui for fabric from this pull it will be in another one

petermetz

I removed gui for fabric from this pull it will be in another one

@barnapa Thank you for that! Now please resolve the merge conflicts and then we can review the actual PR.

I don't know why the other maintainers were not added automatically (we have it configured so that they are but it seems buggy) but I added everyone manually just now as reviewers.

Will wait for the conflict resolution before a full review but I noticed something already that could be addressed at the same time as the conflict resolution:
The tsconfig.base.json and tsconfig.json files were re-formatted which makes it hard to see if there were any actual changes it to so I'll ask to revert the formatting and if you do want to have the tsconfig files in a different format, that can be submitted as another PR which does not change any functionality, only formatting.

This way we don't mix changes that have potentially far reaching consequences (project-wide compiler configuration that affects 40+ packages) and changes that aren't changing anything but are very prone to distracting reviewers from tiny but impactful changes as described above.

packages/cactus-plugin-persistence-fabric/package.json

...ugin-persistence-fabric/src/test/typescript/integration/persistence-fabric-db-client.test.ts

petermetz

@barnapa Please

Update the cacti versions to the current latest (which just had a release yesterday)
Check out my comment about the magic strings

.../cactus-plugin-ledger-connector-fabric/src/main/typescript/plugin-ledger-connector-fabric.ts

.cspell.json

VRamakrishna

I've requested minor edits around terminology and formatting in my comments.

izuru0

LGTM

jagpreetsinghsasan

LGTM

petermetz

@barnapa LGTM

FYI: I've made a couple of changes to the

getRuntimeErrorCause function so that it doesn't hide information about exceptions that are not string nor Error typed
dependencies (removed unused ones, added missing declarations)

1. Add a new plugin for storing hyperledger fabric ledger data into a database 2. Add functional tests for plugin and data access layer operations. operating with fabric-all-in-one docker composition Tests assume any postgres database, but for final deployment postgres or supabase is assumed. Data fed by this plugin can later be visualized by a GUI application or analyzed directly. ( right now the GUI will have switch to operate either with Ethereum ledger or Fabric ledger ) Also upgrade ts-node in the rood package.json because it was causing crashes for the scripts in the ./tools/ directory (such as the webpack bundle name validator script) Depends on: hyperledger-cacti#2259 Depends on: hyperledger-cacti#2265 Co-authored-by: Peter Somogyvari <[email protected]> Signed-off-by: Barnaba Pawełczak <[email protected]> Signed-off-by: Peter Somogyvari <[email protected]>

…rledger ledger data stored in supabase by persistence plugins Depends on: hyperledger-cacti#2265 Depends on: hyperledger-cacti#2331 Signed-off-by: Barnaba Pawelczak [email protected]

barnapa requested review from petermetz, takeutak, izuru0, jagpreetsinghsasan, VRamakrishna and sandeepnRES as code owners March 20, 2023 14:38

petermetz requested changes Mar 28, 2023

View reviewed changes

petermetz mentioned this pull request Mar 28, 2023

feat(cactus-cmd-gui-app): add common cacti gui app #2265

Closed

barnapa force-pushed the cactus-plugin-persistence-fabric-pull-initial branch 2 times, most recently from fc06dff to 2e3c7f1 Compare March 28, 2023 20:07

barnapa requested review from petermetz and removed request for VRamakrishna, sandeepnRES, jagpreetsinghsasan, takeutak and izuru0 March 28, 2023 20:09

petermetz requested changes Mar 30, 2023

View reviewed changes

barnapa force-pushed the cactus-plugin-persistence-fabric-pull-initial branch from 2e3c7f1 to 22213f2 Compare April 3, 2023 06:52

barnapa mentioned this pull request Apr 3, 2023

feat(cactus-cmd-gui-app): extend common cacti gui app to operate with fabric hyperledger #2363

Merged

barnapa requested a review from petermetz April 3, 2023 10:44

barnapa force-pushed the cactus-plugin-persistence-fabric-pull-initial branch from 22213f2 to eda83ce Compare April 3, 2023 13:43

petermetz requested review from VRamakrishna, sandeepnRES, jagpreetsinghsasan, takeutak and izuru0 April 3, 2023 18:09

petermetz requested changes Apr 3, 2023

View reviewed changes