Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat(bungee-hermes): ability to use connectors without instanciating APIs #3295

Merged
merged 1 commit into from
Jul 17, 2024
Merged

feat(bungee-hermes): ability to use connectors without instanciating APIs #3295

merged 1 commit into from
Jul 17, 2024

Conversation

LordKubaya
Copy link
Contributor

BUNGEE-Hermes required API credentials for ledger connectors within its methods. This change enables using Bungee with the connectors themselves.

#3251 (review)

Summary:

  • This includes the implementation to possibilitate the usage of ledgers connectors without instaciating API's with bungee.
  • Tests changed to test for both using connectos and API's.

@LordKubaya LordKubaya changed the title feat(bungee-hermes): ability to use connectors without instanciating … feat(bungee-hermes): ability to use connectors without instanciating APIs Jun 4, 2024
@LordKubaya LordKubaya requested a review from RafaelAPB June 4, 2024 13:23
@LordKubaya LordKubaya requested a review from petermetz July 1, 2024 12:12
@RafaelAPB
Copy link
Contributor

@LordKubaya @AndreAugusto11 what is the status of this PR? Is it good for reviewing?

petermetz
petermetz approved these changes Jul 17, 2024
View reviewed changes
Copy link
Member

@petermetz petermetz left a 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.

RafaelAPB
RafaelAPB approved these changes Jul 17, 2024
View reviewed changes
Copy link
Contributor

@RafaelAPB RafaelAPB left a 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!

@RafaelAPB
Copy link
Contributor

@petermetz are we good to merge?

@LordKubaya
Copy link
Contributor Author

@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.

Thank you @petermetz.
Done.

petermetz
petermetz approved these changes Jul 17, 2024
View reviewed changes
Copy link
Member

@petermetz petermetz left a 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

@petermetz
Copy link
Member

I'll rebase it after #3416 has gone in, please hold off with the updates in the meantime.

@LordKubaya @petermetz
feat(bungee-hermes): ability to use connectors without instanciating …
bcea408 
…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]>
@petermetz petermetz merged commit 6a71ddf into hyperledger:main Jul 17, 2024
140 of 143 checks passed
@petermetz
Copy link
Member

@petermetz are we good to merge?

@RafaelAPB Yup, it's in! :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AndreAugusto11 AndreAugusto11 AndreAugusto11 left review comments

@petermetz petermetz petermetz approved these changes

@RafaelAPB RafaelAPB RafaelAPB approved these changes

@takeutak takeutak Awaiting requested review from takeutak takeutak is a code owner

@izuru0 izuru0 Awaiting requested review from izuru0 izuru0 is a code owner

@jagpreetsinghsasan jagpreetsinghsasan Awaiting requested review from jagpreetsinghsasan jagpreetsinghsasan is a code owner

@VRamakrishna VRamakrishna Awaiting requested review from VRamakrishna VRamakrishna is a code owner

@sandeepnRES sandeepnRES Awaiting requested review from sandeepnRES sandeepnRES is a code owner

@outSH outSH Awaiting requested review from outSH outSH is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@LordKubaya @RafaelAPB @petermetz @AndreAugusto11