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.

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

E2E suite cleanup #904

Merged
merged 8 commits into from
Jul 19, 2022
Merged

E2E suite cleanup #904

merged 8 commits into from
Jul 19, 2022

Conversation

awrichar
Copy link
Contributor

@awrichar awrichar commented Jul 14, 2022

Factor out a struct for FireFlyClient (bringing it closer to being exportable as an actual client SDK).

Reorganize and clean up various pieces between run.sh and the suite itself (preferring to keep run.sh as thin as possible).

awrichar added 5 commits July 14, 2022 13:31
Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
Eventually this could evolve into a proper Go client SDK.

Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
@codecov-commenter
Copy link

codecov-commenter commented Jul 14, 2022

Codecov Report

Merging #904 (d295062) into main (995d53b) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #904      +/-   ##
==========================================
+ Coverage   99.96%   99.97%   +0.01%     
==========================================
  Files         298      298              
  Lines       19563    19572       +9     
==========================================
+ Hits        19557    19568      +11     
+ Misses          5        4       -1     
+ Partials        1        0       -1     
Impacted Files Coverage Δ
internal/identity/identitymanager.go 100.00% <ø> (ø)
cmd/firefly.go 100.00% <100.00%> (ø)
internal/multiparty/manager.go 100.00% <100.00%> (ø)
internal/namespace/manager.go 100.00% <100.00%> (ø)
internal/orchestrator/orchestrator.go 100.00% <100.00%> (ø)
internal/tokens/fftokens/fftokens.go 100.00% <0.00%> (+0.43%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update df58394...d295062. Read the comment docs.

Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
awrichar added 2 commits July 15, 2022 22:59
There is now a general-purpose identityCache for all identities, which takes the
namespace into account (particularly important when resolving V1 identities
between the current namespace and ff_system).

Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
@@ -36,7 +36,7 @@ type Plugin interface {
Init(ctx context.Context, config config.Section, metrics metrics.Manager) error

// SetHandler registers a handler to receive callbacks
// If namespace is set, plugin will attempt to deliver only events for that namespace
// Plugin will attempt (but is not guaranteed) to deliver events only for the given namespace
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this mean that it might accidentally deliver events for a different namespace?

Copy link
Member

@shorsher shorsher Jul 19, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In 1.1, ethconnect/fabconnect subscriptions will be in the format ff-sub-<namespace>-<listener ID>. The plugins will attempt to parse the namespace from the subscription name and then deliver to the correct handler. If not, it will deliver to all plugins.

One scenario where this is likely is existing subscriptions after upgrading to 1.1.

You can see #891 if you're interested.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nguyer In short, yes. The managers that register callbacks must therefore always check each event and discard any that are not relevant to them.

Originally there was no filtering at the plugin level (each plugin would deliver all events it received to every subscribed manager). We added namespace-level filtering as a best effort to make things more performant, but as @shorsher mentioned there are situations where it just isn't tenable at the plugin level.

@shorsher shorsher merged commit 157a0e0 into hyperledger:main Jul 19, 2022
@shorsher shorsher deleted the e2e_cleanup branch July 19, 2022 01:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants