network: Add API for Capabilities #1675
Conversation
network/adaptive_api.go
Outdated
| c := a.get(id) | ||
| if c == nil { | ||
| return false, fmt.Errorf("Capability %d not registered", id) | ||
| } else if idx > len(c.Cap)-1 { |
There was a problem hiding this comment.
No need for else, as the previous if block is returning always.
network/adaptive_api.go
Outdated
| @@ -0,0 +1,45 @@ | |||
| package network | |||
There was a problem hiding this comment.
how about creating a package for capabilities, then we could get rid of all these long names?
There was a problem hiding this comment.
Ok so merge this then I move it to separate package please?
There was a problem hiding this comment.
is this such a big overhead to do now @nolash? we're talking 102 lines delta....
network/adaptive_api.go
Outdated
| @@ -0,0 +1,45 @@ | |||
| package network | |||
There was a problem hiding this comment.
is this such a big overhead to do now @nolash? we're talking 102 lines delta....
network/adaptive_api.go
Outdated
| // RegisterCapability adds the given capability object to the Capabilities collection | ||
| // If the Capability is already registered an error will be returned | ||
| func (a *CapabilitiesAPI) RegisterCapability(cp *Capability) error { | ||
| log.Debug("Registering capability", "cp", cp) |
network/adaptive_api.go
Outdated
| } | ||
|
|
||
| // IsRegisteredCapability returns true if a Capability with the given id is registered | ||
| func (a *CapabilitiesAPI) IsRegisteredCapability(id CapabilityID) (bool, error) { |
There was a problem hiding this comment.
Remember this is in the namespace of bzz from the perspective of RPC callers. Wouldn't it make sense to have the names explicity to avoid potential ambiguity?
network/adaptive_api.go
Outdated
|
|
||
| // MatchCapability returns true if the Capability flag at the given index is set | ||
| // Fails with error if the Capability is not registered, or if the index is out of bounds | ||
| func (a *CapabilitiesAPI) MatchCapability(id CapabilityID, idx int) (bool, error) { |
|
@zelig @acud I have second thoughts about the separate package approach. I understand the reason is to keep method names short. But the idea here was to expose this API under the Would there be other reasons to keep it in a separate package? |
why not have its own RPC namespace too? |
|
To me it seems to belong with |
|
@nolash separating capabilities into a separate package to me is orthogonal of its API namespace. You can still have an exported API type from the package and just hook it up to the API as we do with every other RPC API in Swarm. To me it seems like a compact and isolated enough piece of code that can be isolated into its own package |
* 'master' of github.com:ethersphere/swarm: pss: Modularize crypto and remove Whisper. Step 1 - isolate whisper code (ethersphere#1698) pss: Improve pressure backstop queue handling - no mutex (ethersphere#1695) cmd/swarm-snapshot: if 2 nodes to create snapshot use connectChain (ethersphere#1709) network: Add API for Capabilities (ethersphere#1675) pss: fixed flaky test that was using a global variable instead of a local one (ethersphere#1702) pss: Port tests to `network/simulation` (ethersphere#1682) storage: fix hasherstore seen check to happen when error is nil (ethersphere#1700) vendor: upgrade go-ethereum to 1.9.2 (ethersphere#1689) bzzeth: initial support for bzz-eth protocol (ethersphere#1571) network/stream: terminate runUpdateSyncing on peer quit (ethersphere#1696) all: first working SWAP version (ethersphere#1554) version: update to v0.5.0 unstable (ethersphere#1694) chunk, storage: storage with multi chunk Set method (ethersphere#1684) chunk, storage: add HasMulti to chunk.Store (ethersphere#1686) chunk, shed, storage: chunk.Store GetMulti method (ethersphere#1691) api, chunk: progress bar support (ethersphere#1649)
This PR follows the initial introduction of Capabilities in the network package generally and the Bzz object in particular. It implements some of the API methods outlined in the Adaptive Capabilities SWIP