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

fix: refactor metadata config and change http address to grpc address #170

Merged
merged 6 commits into from
Mar 7, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 1 addition & 9 deletions config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
"os"

"github.com/bnb-chain/greenfield-storage-provider/service/blocksyncer"
"github.com/bnb-chain/greenfield-storage-provider/service/metadata"
"github.com/bnb-chain/greenfield-storage-provider/service/signer"
tomlconfig "github.com/forbole/juno/v4/cmd/migrate/toml"
"github.com/naoina/toml"
Expand All @@ -32,7 +31,6 @@ type StorageProviderConfig struct {
ChainConfig *gnfd.GreenfieldChainConfig
SignerCfg *signer.SignerConfig
BlockSyncerCfg *tomlconfig.TomlConfig
MetadataCfg *metadata.MetadataConfig
LogCfg *LogConfig
}

Expand Down Expand Up @@ -65,7 +63,7 @@ var DefaultStorageProviderConfig = &StorageProviderConfig{
model.SyncerService: model.SyncerGRPCAddress,
model.StoneNodeService: model.StoneNodeGRPCAddress,
model.SignerService: model.SignerGRPCAddress,
model.MetadataService: model.MetaDataServiceGRPCAddress,
model.MetadataService: model.MetaDataGRPCAddress,
},
HTTPAddress: map[string]string{
model.GatewayService: model.GatewayHTTPAddress,
Expand All @@ -77,7 +75,6 @@ var DefaultStorageProviderConfig = &StorageProviderConfig{
ChainConfig: DefaultGreenfieldChainConfig,
SignerCfg: signer.DefaultSignerChainConfig,
BlockSyncerCfg: blocksyncer.DefaultBlockSyncerConfig,
MetadataCfg: DefaultMetadataConfig,
LogCfg: DefaultLogConfig,
}

Expand Down Expand Up @@ -111,11 +108,6 @@ var DefaultGreenfieldChainConfig = &gnfd.GreenfieldChainConfig{
}},
}

var DefaultMetadataConfig = &metadata.MetadataConfig{
Address: model.MetaDataServiceGRPCAddress,
SpDBConfig: DefaultSQLDBConfig,
}

type LogConfig struct {
Level string
Path string
Expand Down
6 changes: 3 additions & 3 deletions config/subconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ func (cfg *StorageProviderConfig) MakeGatewayConfig() (*gateway.GatewayConfig, e
if _, ok := cfg.GRPCAddress[model.MetadataService]; ok {
gCfg.MetadataServiceAddress = cfg.GRPCAddress[model.MetadataService]
} else {
return nil, fmt.Errorf("missing syncer gPRC address configuration for gateway service")
return nil, fmt.Errorf("missing metadata gPRC address configuration for gateway service")
}
return gCfg, nil
}
Expand Down Expand Up @@ -153,7 +153,7 @@ func (cfg *StorageProviderConfig) MakeStoneNodeConfig() (*stonenode.StoneNodeCon
if _, ok := cfg.GRPCAddress[model.SignerService]; ok {
snCfg.SignerGrpcAddress = cfg.GRPCAddress[model.SignerService]
} else {
return nil, fmt.Errorf("missing signer gPRC address configuration for stone node service")
return nil, fmt.Errorf("missing metadata gPRC address configuration for stone node service")
Copy link
Contributor

Choose a reason for hiding this comment

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

signer?

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

}
return snCfg, nil
}
Expand All @@ -164,7 +164,7 @@ func (cfg *StorageProviderConfig) MakeMetadataServiceConfig() (*metadata.Metadat
SpDBConfig: cfg.SpDBConfig,
}
if _, ok := cfg.GRPCAddress[model.MetadataService]; ok {
mCfg.Address = cfg.GRPCAddress[model.MetadataService]
mCfg.GRPCAddress = cfg.GRPCAddress[model.MetadataService]
} else {
return nil, fmt.Errorf("missing meta data HTTP address configuration for mate data service")
}
Expand Down
4 changes: 2 additions & 2 deletions model/const.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,6 @@ var SpServiceDesc = map[string]string{
const (
// GatewayHTTPAddress default HTTP address of gateway
GatewayHTTPAddress = "localhost:9033"
// MetaDataServiceHTTPAddress default HTTP address of meta data service
MetaDataServiceGRPCAddress = "localhost:9733"
// UploaderGRPCAddress default gRPC address of uploader
UploaderGRPCAddress = "localhost:9133"
// DownloaderGRPCAddress default gRPC address of downloader
Expand All @@ -57,6 +55,8 @@ const (
SyncerGRPCAddress = "localhost:9533"
// SignerGRPCAddress default gRPC address of signer
SignerGRPCAddress = "localhost:9633"
// MetaDataGRPCAddress default gRPC address of meta data service
MetaDataGRPCAddress = "localhost:9733"
)

// define greenfield chain default address
Expand Down
2 changes: 1 addition & 1 deletion service/gateway/gateway.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ func NewGatewayService(cfg *GatewayConfig) (*Gateway, error) {

if cfg.MetadataServiceAddress != "" {
if g.metadata, err = client.NewMetadataClient(cfg.MetadataServiceAddress); err != nil {
Copy link
Collaborator

Choose a reason for hiding this comment

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

metadata handler needs to check g.metadata != nil

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i just follow the way of the other services.
image
and it will only return nil when the err occurs(already check err in the above code)
image

Copy link
Collaborator

Choose a reason for hiding this comment

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

If metadata is nil, will metadata related requests(the metadata handler) cause the gateway to crash?

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, add if statement for error response and create errorJSONResponse for response(Clyde said we will provide JSON type in phase 1 api)

log.Warnw("failed to create metadata client", "err", err)
log.Errorw("failed to create metadata client", "err", err)
Copy link
Collaborator

Choose a reason for hiding this comment

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

"err" --> "error"

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

return nil, err
}
}
Expand Down
4 changes: 2 additions & 2 deletions service/metadata/metadata_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,6 @@ import (
)

type MetadataConfig struct {
Address string
SpDBConfig *config.SQLDBConfig
GRPCAddress string
SpDBConfig *config.SQLDBConfig
}
2 changes: 1 addition & 1 deletion service/metadata/service/metadata.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func (metadata *Metadata) Start(ctx context.Context) error {

// Serve starts grpc service.
func (metadata *Metadata) serve(errCh chan error) {
lis, err := net.Listen("tcp", metadata.config.Address)
lis, err := net.Listen("tcp", metadata.config.GRPCAddress)
errCh <- err
if err != nil {
log.Errorw("failed to listen", "err", err)
Expand Down