-
Notifications
You must be signed in to change notification settings - Fork 110
HTTP API support for pinning contents #1658
Conversation
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.
LGTM.
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.
@jmozah I think that the change to expose the private fields on the pin struct is not needed if you implement the MarshalJSON
/UnmarshalJSON
methods (same issue like the other PR). Otherwise LGTM
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.
Just a couple of comments about method and type names in pin.go
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.
approve with minor suggestions to rename a few things
@@ -128,6 +128,10 @@ func (u *URI) Hash() bool { | |||
return u.Scheme == "bzz-hash" | |||
} | |||
|
|||
func (u *URI) Pin() bool { |
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.
comment exported method
isRaw bool | ||
fileSize uint64 | ||
pinCounter uint64 | ||
// PinInfo is the struct that stores the information about pinned files |
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.
should this just be Info. You know, no pkg prefix in names.
or call it Pin
? maybe
type PinInfo struct { | ||
Address storage.Address | ||
IsRaw bool | ||
FileSize uint64 |
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.
Size?
Address storage.Address | ||
IsRaw bool | ||
FileSize uint64 | ||
PinCounter uint64 |
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.
would it not be more intuitive to call this Chunks
?
* 'master' of github.com:ethersphere/swarm: chunk, storage: chunk.Store multiple chunk Put (ethersphere#1681) storage: fix pyramid chunker and hasherstore possible deadlocks (ethersphere#1679) pss: Use distance to determine single guaranteed recipient (ethersphere#1672) storage: fix possible hasherstore deadlock on waitC channel (ethersphere#1674) network: Add adaptive capabilities message (ethersphere#1619) p2p/protocols, p2p/testing; conditional propagation of context (ethersphere#1648) api/http: remove unnecessary conversion (ethersphere#1673) storage: fix LazyChunkReader.join potential deadlock (ethersphere#1670) HTTP API support for pinning contents (ethersphere#1658) pot: Add Distance methods with tests (ethersphere#1621) README.md: Update Vendored Dependencies section (ethersphere#1667) network, p2p, vendor: move vendored p2p/testing under swarm (ethersphere#1647) build, vendor: use go modules for vendoring (ethersphere#1532)
Three HTTP API related to pinning is added
bin-pin:/?IsRaw=true to pin a raw file
bin-pin:/ to pin a collection