feat(connector-besu, connector-quorum): filesystem replaced by keychain #702
Conversation
There was a problem hiding this comment.
@jordigiam When we generate the changelogs at release time, with the current commit message, this will show up as besu getting the keychain support, but there will be nothing there about quorum. Is there any chance that you could either break it up into two commits (it's OK to keep the PR as is, with 2 commits because they are doing the same change anyway) like
Commit 1 feat(connector-besu): filesystem replaced by keychain
Commit 2 feat(connector-quorum): filesystem replaced by keychain
Or you could also leave it in one commit and just edit the scope to include both besu and quorum like
feat(connector-besu,connector-quorum): filesystem replaced by keychain
Assuming it's not over the character limit this would work as well. Also any other solution that makes sure that the generated changelogs will mention both quorum and besu would work for me, but I only know of the two options above to make it so.
| ); | ||
| if (contractJson.contractName != contractName) { | ||
| if (!keychainPlugin.has(contractName)) { | ||
| throw new Error( | ||
| `${fnTag} Cannot create an instance of the contract because the contractName and the contractName of the JSON doesn't match`, |
There was a problem hiding this comment.
@jordigiam Is it possible that the error message of the exception is no longer accurate as to what is happening? I'd slightly rephrase it to explain that the request had a contract name specified and we did not find an entry like that in the keychain.
jordigiam
force-pushed
the
feature/replace-filesystem
branch
from
9cf7c80 to
96ab837
Compare
jordigiam
changed the title
There was a problem hiding this comment.
@jordigiam Just tracking/noting down for the record here what we discussed on the pair programming session today => please make sure that the request can always override whatever is in this.contracts by using the keychain instead.
jordigiam
force-pushed
the
feature/replace-filesystem
branch
4 times, most recently
from
8246edc to
604edf6
Compare
Signed-off-by: jordigiam <jordi.giron.amezcua@accenture.com>
jordigiam
force-pushed
the
feature/replace-filesystem
branch
from
604edf6 to
6cdc0c3
Compare
With these changes I was able to build the container image
and then launch it, test it, etc.
The Fabric part of it fails with the error below which I
suspect is a problem with Fabric changing their
release tags over time, breaking software that depends
on Fabric even when exact versions are specified.
},
data: {
error: 'Error: PluginLedgerConnectorFabric#transact() Unable to run transaction: Event strategy not satisfied within the timeout period. No response received from event hubs: peer0.org1.example.com, peer0.org2.example.com:9051, peer1.org2.example.com:10051, peer1.org1.example.com:8051\n' +
' at PluginLedgerConnectorFabric.transact (/cactus-scratch-1/packages/cactus-plugin-ledger-connector-fabric/dist/lib/main/typescript/plugin-ledger-connector-fabric.js:331:19)\n' +
' at runMicrotasks (<anonymous>)\n' +
' at processTicksAndRejections (internal/process/task_queues.js:97:5)'
}
Signed-off-by: Peter Somogyvari <peter.somogyvari@accenture.com>
There was a problem hiding this comment.
LGTM
Note to self: We need to put in some time to design something that's stateless so that the contracts map can be eliminated entirely (somehow, but I don't know any better right now either except for the consortium database, but that's also not a mature design for contract storage so back to my initial point about putting time into some design...)
One thing I noticed is that we could expose a get network ID endpoint on the web3 flavored connectors (besu, quorum) so that we can build on that in the future by requiring the caller to fill the network ID themselves and freeing the connector from having to do that altogether.
Resolves #678
Resolves #680