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

[chain_simulator] Enable http server option #5699

Merged
merged 8 commits into from Nov 14, 2023

Conversation

miiu96
Copy link
Contributor

@miiu96 miiu96 commented Nov 9, 2023

Reasoning behind the pull request

Pre-requisites

Based on the Contributing Guidelines the PR author and the reviewers must check the following requirements are met:

  • was the PR targeted to the correct branch?
  • if this is a larger feature that probably needs more than one PR, is there a feat branch created?
  • if this is a feat branch merging, do all satellite projects have a proper tag inside go.mod?

@miiu96 miiu96 changed the title enable rest api interface [chain_simulator] Enable http server option Nov 9, 2023
Copy link

codecov bot commented Nov 9, 2023

Codecov Report

Attention: 259 lines in your changes are missing coverage. Please review.

Comparison is base (131a29a) 79.06% compared to head (15f81f1) 78.89%.
Report is 1 commits behind head on feat/test-only-processor-node.

❗ Current head 15f81f1 differs from pull request most recent head 90bf06f. Consider uploading reports for the commit 90bf06f to get more accurate results

Files Patch % Lines
node/chainSimulator/components/nodeFacade.go 0.00% 121 Missing ⚠️
...hainSimulator/components/testOnlyProcessingNode.go 0.00% 48 Missing ⚠️
.../chainSimulator/components/statusCoreComponents.go 0.00% 28 Missing ⚠️
node/chainSimulator/components/statusComponents.go 0.00% 11 Missing ⚠️
node/chainSimulator/components/cryptoComponents.go 0.00% 9 Missing ⚠️
...e/chainSimulator/components/bootstrapComponents.go 0.00% 7 Missing ⚠️
node/chainSimulator/components/coreComponents.go 0.00% 7 Missing ⚠️
node/chainSimulator/components/dataComponents.go 0.00% 7 Missing ⚠️
...ode/chainSimulator/components/networkComponents.go 0.00% 7 Missing ⚠️
...ode/chainSimulator/components/processComponents.go 0.00% 7 Missing ⚠️
... and 1 more
Additional details and impacted files
@@                        Coverage Diff                        @@
##           feat/test-only-processor-node    #5699      +/-   ##
=================================================================
- Coverage                          79.06%   78.89%   -0.18%     
=================================================================
  Files                                728      729       +1     
  Lines                              95517    95721     +204     
=================================================================
- Hits                               75519    75516       -3     
- Misses                             14638    14846     +208     
+ Partials                            5360     5359       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@@ -38,7 +38,7 @@ type bootstrapComponentsHolder struct {
}

// CreateBootstrapComponentHolder will create a new instance of bootstrap components holder
func CreateBootstrapComponentHolder(args ArgsBootstrapComponentsHolder) (factory.BootstrapComponentsHolder, error) {
func CreateBootstrapComponentHolder(args ArgsBootstrapComponentsHolder) (factory.BootstrapComponentsHandler, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please rename to CreateBootstrapComponents and remove the inner closeHandler declaration & usage

Copy link
Contributor Author

Choose a reason for hiding this comment

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

renamed

@@ -46,7 +46,7 @@ type cryptoComponentsHolder struct {
}

// CreateCryptoComponentsHolder will create a new instance of cryptoComponentsHolder
func CreateCryptoComponentsHolder(args ArgsCryptoComponentsHolder) (factory.CryptoComponentsHolder, error) {
func CreateCryptoComponentsHolder(args ArgsCryptoComponentsHolder) (factory.CryptoComponentsHandler, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please rename to CreateCryptoComponents

Copy link
Contributor Author

Choose a reason for hiding this comment

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

renamed

@@ -25,7 +25,7 @@ type dataComponentsHolder struct {
}

// CreateDataComponentsHolder will create the data components holder
func CreateDataComponentsHolder(args ArgsDataComponentsHolder) (factory.DataComponentsHolder, error) {
func CreateDataComponentsHolder(args ArgsDataComponentsHolder) (factory.DataComponentsHandler, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please rename to CreateDataComponents and remove the inner closeHandler declaration & usage

Copy link
Contributor Author

Choose a reason for hiding this comment

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

renamed

@@ -27,7 +27,7 @@ type networkComponentsHolder struct {
}

// CreateNetworkComponentsHolder creates a new networkComponentsHolder instance
func CreateNetworkComponentsHolder(network SyncedBroadcastNetworkHandler) (*networkComponentsHolder, error) {
func CreateNetworkComponentsHolder(network SyncedBroadcastNetworkHandler) (factory.NetworkComponentsHandler, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please rename to CreateNetworkComponents and remove the inner closeHandler declaration & usage

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

node/chainSimulator/components/nodeFacade.go Show resolved Hide resolved
@@ -31,7 +31,7 @@ type stateComponentsHolder struct {
}

// CreateStateComponents will create the state components holder
func CreateStateComponents(args ArgsStateComponents) (factory.StateComponentsHolder, error) {
func CreateStateComponents(args ArgsStateComponents) (factory.StateComponentsHandler, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please rename to CreateStateComponents and remove the inner closeHandler declaration & usage

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@@ -24,25 +19,41 @@ type statusCoreComponentsHolder struct {
}

// CreateStatusCoreComponentsHolder will create a new instance of factory.StatusCoreComponentsHolder
func CreateStatusCoreComponentsHolder(cfg config.Config, coreComponents factory.CoreComponentsHolder) (factory.StatusCoreComponentsHolder, error) {
func CreateStatusCoreComponentsHolder(configs config.Configs, coreComponents factory.CoreComponentsHolder) (factory.StatusCoreComponentsHandler, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please rename to CreateStatusCoreComponents and remove the inner closeHandler declaration & usage

Copy link
Contributor Author

Choose a reason for hiding this comment

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

renamed

@@ -22,6 +23,8 @@ import (

// ArgsTestOnlyProcessingNode represents the DTO struct for the NewTestOnlyProcessingNode constructor function
type ArgsTestOnlyProcessingNode struct {
Configs config.Configs
// TODO remove the rest of configs because configs contains all of them
Copy link
Contributor

Choose a reason for hiding this comment

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

implement TODO now or in the next PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done.

@@ -326,7 +342,11 @@ func (node *testOnlyProcessingNode) GetStateComponents() factory.StateComponents
return node.StateComponentsHolder
}

func (node *testOnlyProcessingNode) collectClosableComponents() {
func (node *testOnlyProcessingNode) GetFacadeHandler() shared.FacadeHandler {
Copy link
Contributor

Choose a reason for hiding this comment

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

missing comment

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@@ -335,6 +355,10 @@ func (node *testOnlyProcessingNode) collectClosableComponents() {
node.closeHandler.AddComponent(node.NetworkComponentsHolder)
node.closeHandler.AddComponent(node.StatusCoreComponents)
node.closeHandler.AddComponent(node.CoreComponentsHolder)
node.closeHandler.AddComponent(node.facadeHandler)
if enableHTTPServer {
Copy link
Contributor

Choose a reason for hiding this comment

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

instead of this ugly if, the createHttpServer can return a mock http server if the chain simulator is run without APIs

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

@@ -31,6 +31,7 @@ func NewChainSimulator(
genesisTimestamp int64,
roundDurationInMillis uint64,
roundsPerEpoch core.OptionalUint64,
apiInterface components.APIConfigurator, // interface
Copy link
Contributor

Choose a reason for hiding this comment

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

remove // interface ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@@ -11,3 +11,7 @@ type SyncedBroadcastNetworkHandler interface {
GetConnectedPeersOnTopic(topic string) []core.PeerID
IsInterfaceNil() bool
}

type APIConfigurator interface {
Copy link
Contributor

Choose a reason for hiding this comment

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

missing comments

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@ssd04 ssd04 self-requested a review November 13, 2023 11:10

// RestApiInterface will return the api interface for the provided shard
func (f *fixedPortAPIConfigurator) RestApiInterface(shardID uint32) string {
return fmt.Sprintf("%s:%d", f.restAPIInterface, f.restAPIInterface[shardID])
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return fmt.Sprintf("%s:%d", f.restAPIInterface, f.restAPIInterface[shardID])
return fmt.Sprintf("%s:%d", f.restAPIInterface, f.mapShardPort[shardID])

?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed


// StartPolling will do nothing
func (s *statusComponentsHolder) StartPolling() error {
// todo check if this method
Copy link
Contributor

Choose a reason for hiding this comment

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

solve or update this todo message

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 todo for the monent

@@ -23,26 +18,45 @@ type statusCoreComponentsHolder struct {
persistentStatusHandler factory.PersistentStatusHandler
}

// CreateStatusCoreComponentsHolder will create a new instance of factory.StatusCoreComponentsHolder
func CreateStatusCoreComponentsHolder(cfg config.Config, coreComponents factory.CoreComponentsHolder) (factory.StatusCoreComponentsHolder, error) {
// CreateStatusCoreComponents will create a new instance of factory.StatusCoreComponentsHolder
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// CreateStatusCoreComponents will create a new instance of factory.StatusCoreComponentsHolder
// CreateStatusCoreComponents will create a new instance of factory.StatusCoreComponentsHandler

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@@ -17,5 +17,5 @@ func NewFixedPortAPIConfigurator(restAPIInterface string, mapShardPort map[uint3

// RestApiInterface will return the api interface for the provided shard
func (f *fixedPortAPIConfigurator) RestApiInterface(shardID uint32) string {
return fmt.Sprintf("%s:%d", f.restAPIInterface, f.restAPIInterface[shardID])
return fmt.Sprintf("%s:%d", f.restAPIInterface, f.mapShardPort[shardID])
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@miiu96 miiu96 merged commit a050b92 into feat/test-only-processor-node Nov 14, 2023
5 checks passed
@miiu96 miiu96 deleted the enable-http-server branch November 14, 2023 12:58
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.

None yet

3 participants