-
Notifications
You must be signed in to change notification settings - Fork 242
Re-subscribe if the instance path changes #256
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
Conversation
Signed-off-by: Peter Broadhurst <peter.broadhurst@kaleido.io>
Signed-off-by: Peter Broadhurst <peter.broadhurst@kaleido.io>
Signed-off-by: Peter Broadhurst <peter.broadhurst@kaleido.io>
Signed-off-by: Peter Broadhurst <peter.broadhurst@kaleido.io>
Codecov Report
@@ Coverage Diff @@
## main #256 +/- ##
==========================================
- Coverage 99.55% 99.53% -0.03%
==========================================
Files 217 217
Lines 12181 12185 +4
==========================================
+ Hits 12127 12128 +1
- Misses 40 42 +2
- Partials 14 15 +1
Continue to review full report at Codecov.
|
Signed-off-by: Peter Broadhurst <peter.broadhurst@kaleido.io>
nguyer
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like there was a slight drop in coverage. Other than that, looks good.
internal/config/config.go
Outdated
|
|
||
| // GetKnownKeys gets the known keys | ||
| func GetKnownKeys() []string { | ||
| var keys []string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this tends to be called a lot, one improvement to consider is to create keys to be the length of knownKeys instead of appending.
keys := make([]string, len(knownKeys))
...
idx := 0
for k := range knownKeys {
keys[idx++] = k
}
|
The drop in coverage is in |
Signed-off-by: Peter Broadhurst <peter.broadhurst@kaleido.io>
For newly created environments, this PR adds a 16 character unique suffix to the ethconnect subscription names it creates, using the instance path from the config at the time the sub is created. This means that if the contract instance changes in the future, we will re-subscribe to the new contract.
Note I also saw this issue in a test run on my machine, and added a change to read this config once on HTTP server startup.