Added custom ApplicationSetting so apps can add their on configuration values #29
Conversation
| edgexSdk.LoggingClient.Error("ApplicationName application settings not found") | ||
| } | ||
| } else { | ||
| edgexSdk.LoggingClient.Error("No application settings not found") |
There was a problem hiding this comment.
For the example I didn't think it was necessary to return so that the example will still run.
There was a problem hiding this comment.
Adding exit... ;-)
| if ok { | ||
| edgexSdk.LoggingClient.Info(fmt.Sprintf("%s now running...", appName)) | ||
| } else { | ||
| edgexSdk.LoggingClient.Error("ApplicationName application settings not found") |
There was a problem hiding this comment.
For the example I didn't think it was necessary to return so that the example will still run.
examples/simple-filter-xml/main.go
Outdated
| if ok { | ||
| edgexSdk.LoggingClient.Info(fmt.Sprintf("%s now running...", appName)) | ||
| } else { | ||
| edgexSdk.LoggingClient.Error("ApplicationName application settings not found") |
There was a problem hiding this comment.
For the example I didn't think it was necessary to return so that the example will still run.
examples/simple-filter-xml/main.go
Outdated
| edgexSdk.LoggingClient.Error("ApplicationName application settings not found") | ||
| } | ||
| } else { | ||
| edgexSdk.LoggingClient.Error("No application settings not found") |
There was a problem hiding this comment.
For the example I didn't think it was necessary to return so that the example will still run.
| sdk := AppFunctionsSDK {} | ||
|
|
||
| sdk.configDir = "../../examples/simple-filter-xml/res" | ||
| err := sdk.initializeConfiguration() |
There was a problem hiding this comment.
Not sure I'm fond of a unit test relying on the file system...
There was a problem hiding this comment.
No much choice here since it needs to load the toml file.
There was a problem hiding this comment.
Is this something that really needs to be unit tested? It's bootstrapping logic, not business logic. It's like unit testing a DI container. Is there going to be a mock RegistryClient added too then to cover if sdk.useRegistry {?
There was a problem hiding this comment.
The purpose was to validate that the new custom section in the toml file was un-marshed as expected. So useRegistry is out of scope.
There was a problem hiding this comment.
BTW, the test is for sdk.ApplicationSettings(). Calling initilizeConfiguration() is part of the test setup.
There was a problem hiding this comment.
@tsconn23 , FYI, the device-sdk-go is doing similar load from local file for their test.
https://github.com/edgexfoundry/device-sdk-go/blob/master/internal/config/loader_test.go#L113
I choose to not create a local test toml file since the one in the example has what is needed.
| edgexSdk.LoggingClient.Error("ApplicationName application settings not found") | ||
| } | ||
| } else { | ||
| edgexSdk.LoggingClient.Error("No application settings not found") |
There was a problem hiding this comment.
Double negative :) "No Application settings not found"?
lenny-goodell
force-pushed
the
CustomConfig
branch
from
1240480
to
d38ebed
Compare
|
recheck |
|
recheck |
1 similar comment
|
recheck |
lenny-goodell
force-pushed
the
CustomConfig
branch
from
d38ebed
to
88acf93
Compare
lenny-goodell
force-pushed
the
CustomConfig
branch
from
5c2d8d2
to
5213ad6
Compare
…n values Signed-off-by: lenny-intel <leonard.goodell@intel.com>
|
Closing PR due to issues and will re-create. |
No description provided.