Strip down namespace object and clean up APIs #898
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
awrichar
requested review from
peterbroadhurst,
nguyer,
shorsher and
nickgaski
as code owners
Codecov Report
@@ Coverage Diff @@
## main #898 +/- ##
=======================================
Coverage 99.97% 99.98%
=======================================
Files 298 297 -1
Lines 19473 19400 -73
=======================================
- Hits 19469 19397 -72
+ Misses 4 3 -1
|
Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
Although it's stored on the namespace object, this is more appropriate as a piece of the namespace status (next to the list of plugins, etc). Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
Keep it separate from the user-configured values. Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
Namespaces will no longer store an ID, type, or message ID in the database. They will no longer emit SPI events. The /status API will also return all namespace details as an object, instead of just returning a name. Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
awrichar
force-pushed
the
status
branch
5 times, most recently
from
6471067
to
f6a07a8
Compare
Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
awrichar
force-pushed
the
status
branch
2 times, most recently
from
9ce561c
to
0d2a461
Compare
shorsher
reviewed
Jul 20, 2022
| // NodeStatusPlugin field descriptions | ||
| NodeStatusPluginName = ffm("NodeStatusPlugin.name", "The name of the plugin") | ||
| NodeStatusPluginType = ffm("NodeStatusPlugin.pluginType", "The type of the plugin") | ||
| NamespaceID = ffm("Namespace.id", "The UUID of the namespace. For locally established namespaces will be different on each node in the network. For broadcast namespaces, will be the same on every node") |
There was a problem hiding this comment.
Do we need to change these descriptions to get rid of the broadcast terminology?
shorsher
reviewed
Jul 20, 2022
| case stored != nil: | ||
| stored.RemoteName = ns.remoteName | ||
| stored.Description = ns.description | ||
| // TODO: should we check for discrepancies in the multiparty contract config? |
There was a problem hiding this comment.
Should we do this and return an error? At a minimum, maybe we add a log statement that we're overwriting a stored config that has differences.
shorsher
reviewed
Jul 20, 2022
shorsher
reviewed
Jul 20, 2022
| PluginType string `ffstruct:"NamespaceStatusPlugin" json:"pluginType"` | ||
| } | ||
|
|
||
| type NamespaceStatusMultiparty struct { |
There was a problem hiding this comment.
Does this struct need a comment since it's exported?
shorsher
approved these changes
Jul 20, 2022
|
Merging this, made a task on the board to revisit these comments before release. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Part of FIR-12
In a chain with #895Remove all remaining functionality related to broadcasting namespaces. It was already broken from previous commits, but this is now to confirm:
Clean up the
/namespaces/{ns}and/namespaces/{ns}/statusAPIs to be a little more organized.Clean up all namespace database operations to happen from Namespace Manager. Pass a pointer to the namespace object to any other managers that need it.