Dummy service which serves Noop tunnel #506
Conversation
services/noop/service.go
Outdated
| sessionConfigProvider = func() (session.ServiceConfiguration, error) { | ||
| return nil, nil | ||
| } | ||
| log.Info(logPrefix, "Openvpn service started successfully") |
| } | ||
|
|
||
| // Wait blocks until service is stopped | ||
| func (manager *Manager) Wait() error { |
There was a problem hiding this comment.
This breaks contract. Service is started ( starts return nil - no error), then wait should block until stop is called. I can be simply implemented with channels or wait groups.
| } | ||
|
|
||
| // Stop stops service | ||
| func (manager *Manager) Stop() error { |
core/service/stub_service.go
Outdated
| func (service *serviceFake) Start(identity.Identity) (dto_discovery.ServiceProposal, session.ConfigProvider, error) { | ||
| proposal := dto_discovery.ServiceProposal{} | ||
|
|
||
| if service.onStartReturnError != nil { |
There was a problem hiding this comment.
It can be just return proposal, nil, service.onStartReturnError
services/noop/discovery_dto.go
Outdated
| // PaymentMethodNoop defines payment method wo payment at all | ||
| const PaymentMethodNoop = "NOOP" | ||
|
|
||
| // PaymentNoop structure |
There was a problem hiding this comment.
Should it a private structure? In this case, we can avoid such meaningless comments.
core/connection/manager.go
Outdated
| @@ -1,5 +1,5 @@ | |||
| /* | |||
| * Copyright (C) 2017 The "MysteriumNetwork/node" Authors. | |||
| * Copyright (C) 2018 The "MysteriumNetwork/node" Authors. | |||
There was a problem hiding this comment.
If we are changing it without changing the file, let's do it for every file at once.
There was a problem hiding this comment.
in a separate pr, preferably
There was a problem hiding this comment.
Reverted, it was a mistake
core/service/registry.go
Outdated
| return nil, ErrUnsupportedServiceType | ||
| } | ||
|
|
||
| return createConnection(options) |
There was a problem hiding this comment.
createConnection returns service? Maybe createService?
services/noop/service.go
Outdated
| err error, | ||
| ) { | ||
| proposal = dto_discovery.ServiceProposal{ | ||
| ServiceType: "noop", |
There was a problem hiding this comment.
We could have var serviceType = "noop" constant, which can be reused.
core/connection/manager.go
Outdated
| @@ -1,5 +1,5 @@ | |||
| /* | |||
| * Copyright (C) 2017 The "MysteriumNetwork/node" Authors. | |||
| * Copyright (C) 2018 The "MysteriumNetwork/node" Authors. | |||
There was a problem hiding this comment.
in a separate pr, preferably
| func (registry *Registry) Create(options Options) (Service, error) { | ||
| createConnection, exists := registry.factories[options.Type] | ||
| if !exists { | ||
| return nil, ErrUnsupportedServiceType |
There was a problem hiding this comment.
it would be nice to return an error containing the type that was specified, not just an error
There was a problem hiding this comment.
Not my scope. Package "core/connection" and "core/service" already has bunch of errors standardised thru constants.
Feel free to improve it if You see how ;)
| @@ -21,6 +21,7 @@ package service | |||
| type Options struct { | |||
| Identity string | |||
| Passphrase string | |||
| Type string | |||
There was a problem hiding this comment.
we could also use an enum here instead of a string, since we're probably gonna have a limited number of service types.
There was a problem hiding this comment.
Dont agree here. It's hook-able transports so central Core does not know predefined list
There was a problem hiding this comment.
Plus this structure is from user input, so it's free text - so that it could be validated later
Waldz
force-pushed
the
feature/noop-service
branch
from
bfd9fb2 to
1eabae5
Compare
core/service/stub_service.go
Outdated
| func (service *serviceFake) Start(identity.Identity) (dto_discovery.ServiceProposal, session.ConfigProvider, error) { | ||
| proposal := dto_discovery.ServiceProposal{} | ||
|
|
||
| if service.onStartReturnError != nil { |
Updates #382