Skip to content

refactor in router#635

Open
DorKatzelnick wants to merge 1 commit intohyperledger:mainfrom
DorKatzelnick:routerRefactor1
Open

refactor in router#635
DorKatzelnick wants to merge 1 commit intohyperledger:mainfrom
DorKatzelnick:routerRefactor1

Conversation

@DorKatzelnick
Copy link
Contributor

refactor in router, prepare for dynamic reconfig

}

func NewRouter(config *nodeconfig.RouterNodeConfig, logger types.Logger, signer identity.SignerSerializer, configUpdateProposer policy.ConfigUpdateProposer, configRulesVerifier verify.OrdererRules) *Router {
func NewRouter(config *nodeconfig.RouterNodeConfig, configuration *config.Configuration, logger types.Logger, signer identity.SignerSerializer, configUpdateProposer policy.ConfigUpdateProposer, configRulesVerifier verify.OrdererRules) *Router {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is redundant, no? the RouterNodeConfig is extracted from configuration *config.Configuration, right?
It seems we can move the

routerConf := conf.ExtractRouterConfig(lastConfigBlock)

into the NewRouter, maybe adding an error there for returning an error and panic in the launch?
Seems like this can be done to all server roles?
I also like the way the router starts the service on the router instance instead of doing it in the launch like the batcher, for example.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, the RouterNodeConfig object is redundant. for now we need both to apply the new config update.
I think that it is better to remove the dependency on the RouterNodeConfig, since it requires some refactoring.

configRulesVerifier verify.OrdererRules
}

func CreateConfigSubmitter(conf *nodeconfig.RouterNodeConfig, logger types.Logger, verifier *requestfilter.RulesVerifier, signer identity.SignerSerializer, configUpdateProposer policy.ConfigUpdateProposer, configRulesVerifier verify.OrdererRules) *configSubmitter {
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems Create* calls New* and that is it; why not just change the New* ?
New* is now only use in tests, so you can probably change the test to take Create* or the modified New*, right?
This is confusing...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed the create

Signed-off-by: Dor.Katzelnick <Dor.Katzelnick@ibm.com>
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