Skip to content

Moved service registration in final SDK from base SDKs #786

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

Open
wants to merge 7 commits into
base: f-remove-sp
Choose a base branch
from

Conversation

alexandrosfilios
Copy link
Contributor

@alexandrosfilios alexandrosfilios commented Mar 21, 2025

This PR enables us to write application without the need for the Service Provider. To this end:

  • it removes the mandatory (but unnecessary dependencies from the service provider). Instead, the specific services are injected to the node and the view manager
  • for almost all integration tests, it removes the service registrations from the core SDKs (view, fabric, Orion) to the end SDK's (ATSA, CC, etc.)
  • for the IOU integration test, it introduces as a guide the new way of writing views using DI instead of the service provider.

@alexandrosfilios alexandrosfilios force-pushed the f-view-factories branch 4 times, most recently from dee6a7e to 1ba5fc1 Compare March 25, 2025 13:44
@alexandrosfilios alexandrosfilios marked this pull request as ready for review March 25, 2025 14:17
Signed-off-by: Alexandros Filios <[email protected]>
Signed-off-by: Alexandros Filios <[email protected]>
Signed-off-by: Alexandros Filios <[email protected]>
Signed-off-by: Alexandros Filios <[email protected]>
@alexandrosfilios alexandrosfilios changed the base branch from main to f-remove-sp March 25, 2025 16:51
Signed-off-by: Alexandros Filios <[email protected]>
Signed-off-by: Alexandros Filios <[email protected]>
Signed-off-by: Alexandros Filios <[email protected]>
Copy link
Contributor

@adecaro adecaro left a comment

Choose a reason for hiding this comment

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

please, remove the panics.

@@ -23,6 +23,9 @@ var (
logger = logging.MustGetLogger("fabric-sdk")
)

const DefaultNetwork = ""
Copy link
Contributor

Choose a reason for hiding this comment

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

please, add as single block of constants and give a description of the them

@@ -36,10 +36,10 @@ func Wrap(tx *endorser.Transaction) (*Transaction, error) {

// NewTransaction returns a new instance of a state-based transaction that embeds a single namespace.
func NewTransaction(context view.Context) (*Transaction, error) {
return newTransaction(utils.MustGet(fabric.GetNetworkServiceProvider(context)), context)
return NewTransactionWithFNSP(utils.MustGet(fabric.GetNetworkServiceProvider(context)), context)
Copy link
Contributor

Choose a reason for hiding this comment

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

please, don't panic here, check what's the output of fabric.GetNetworkServiceProvider and return an error.

@@ -211,6 +216,9 @@ func ExchangeRecipientIdentities(context view.Context, recipient view.Identity,
Network: opt.Network,
Other: recipient,
IdentityLabel: opt.Identity,

fnsProvider: utils.MustGet(fabric.GetNetworkServiceProvider(context)),
Copy link
Contributor

Choose a reason for hiding this comment

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

plase, don't panic, check the output of fabric.GetNetworkServiceProvider.

finality: o.finality,
timeout: o.timeout,
fnsProvider: utils.MustGet(fabric.GetNetworkServiceProvider(ctx)),
finalityView: NewFinalityViewFactory(utils.MustGet(fabric.GetNetworkServiceProvider(ctx))),
Copy link
Contributor

Choose a reason for hiding this comment

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

please, don't panic.

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.

2 participants