-
Notifications
You must be signed in to change notification settings - Fork 285
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(bungee-hermes): ability to use connectors without instanciating APIs #3295
Conversation
packages/cactus-plugin-bungee-hermes/src/main/typescript/strategy/strategy-besu.ts
Outdated
Show resolved
Hide resolved
packages/cactus-plugin-bungee-hermes/src/main/typescript/strategy/strategy-besu.ts
Show resolved
Hide resolved
packages/cactus-plugin-bungee-hermes/src/main/typescript/strategy/strategy-besu.ts
Outdated
Show resolved
Hide resolved
...ages/cactus-plugin-bungee-hermes/src/test/typescript/integration/fabric-test-pruning.test.ts
Show resolved
Hide resolved
841ac13
to
f7f9bb6
Compare
f0e959b
to
41ff05f
Compare
packages/cactus-plugin-bungee-hermes/src/main/typescript/strategy/strategy-ethereum.ts
Show resolved
Hide resolved
@LordKubaya @AndreAugusto11 what is the status of this PR? Is it good for reviewing? |
41ff05f
to
0148801
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.
@LordKubaya LGMT with comments:
Please fix the missing auto-formatting: If you run yarn lint
locally and then commit the formatting changes it makes then the CI should pass.
Source is the yarn lint CI job failure message:
Error: yarn lint script produced version control side-effects: source files have been changed by it that are otherwise are under version control. This means (99% of the time) that you need to run the yarn lint script locally and then include the changes it makes in your own commit when submitting your pull request.
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 contribution!
@petermetz are we good to merge? |
0148801
to
390e2ff
Compare
Thank you @petermetz. |
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.
@LordKubaya Nice! Thank you very much! LGTM
I'll rebase it after https://github.com/hyperledger/cacti/pull/3416 has gone in, please hold off with the updates in the meantime. |
…APIs Signed-off-by: Carlos Amaro <[email protected]> refactor(bungee): besu strategy error handling and input validation Notice a few things: 1. The http-errors-enhanced-cjs library is being used to encode information about whether an error is consdiered (by us) as a user error or a bug in our code (e.g. it signals who do we think are at fault which is the important question when handling errors.). If we think the input was valid an we should hae succeeded with the function execution, then we throw an InternalServerError. Otherwise if we think the input was not in the required format, we throw a BadRequestError notifying the user (or developer who is calling our function) that they need to fix their input that they provided us with. 2. The error handling code and the output validation code are flattened out as much as possible so that it's easier to read and understand what the code does in what scenario and as the execution progresses the possible bad situations are slowly but surely eliminated and by the end we are confident that the output was in the expected format and that we can return it as-is and we don't need to have any type casting at all. 3. I also added the `pluginRegistry` parameter to the bungee plugin's options because this is an object that the API server passes in to every plugin that it instanties which is meant to facilitate the cross-plugin interaction, e.g. you can use in each plugin the passed in pluginRegistry to look up other plugins instances directly and invoke them without needing an API client object for doing so (which also adds a huge overhead of network latency) The relevant piece of code to be aware of to see why this is working is in the API server class, as pasted below where you can see that the pluginRegistry gets appended to all the plugin options regardless of which kind of plugin it is: ```typescript private async instantiatePlugin( pluginImport: PluginImport, registry: PluginRegistry, ): Promise<ICactusPlugin> { const fnTag = `${this.className}#instantiatePlugin`; const { logLevel } = this.options.config; const { packageName, options } = pluginImport; this.log.info(`Creating plugin from package: ${packageName}`, options); const pluginOptions = { ...options, logLevel, pluginRegistry: registry }; ``` Signed-off-by: Peter Somogyvari <[email protected]> refactor(bungee): besu, ethereum and fabric strategy error handling and input validation Signed-off-by: Carlos Amaro <[email protected]>
390e2ff
to
bcea408
Compare
@RafaelAPB Yup, it's in! :-) |
BUNGEE-Hermes required API credentials for ledger connectors within its methods. This change enables using Bungee with the connectors themselves.
https://github.com/hyperledger/cacti/pull/3251#pullrequestreview-2077548976
Summary: