-
Notifications
You must be signed in to change notification settings - Fork 44
feat: added support for pinning service configs #104
Conversation
@@ -19,7 +19,7 @@ var DefaultBootstrapAddresses = []string{ | |||
"/dnsaddr/bootstrap.libp2p.io/p2p/QmQCU2EcMqAqQPR2i9bChDtGNJchTbq5TbXJJ16u19uLTa", | |||
"/dnsaddr/bootstrap.libp2p.io/p2p/QmbLHAnMoJPWSCR5Zhtx6BHJX9KiKNN6tpvbUcqanj75Nb", | |||
"/dnsaddr/bootstrap.libp2p.io/p2p/QmcZf59bWwK5XFi76CZX8cbJ4BhTzzA3gU1ZjYZcYW3dwt", | |||
"/ip4/104.131.131.82/tcp/4001/p2p/QmaCpDMGvV2BGHeYERUEnRQAwe3N8SzbUtfsmvsqQLuvuJ", // mars.i.ipfs.io |
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.
previously missed go fmt
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.
Some questions inline
pinning.go
Outdated
// Pinning configures the pinning services. | ||
type Pinning struct { | ||
// Services lists the pinning services | ||
Services []PinningServices |
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.
@aschmahmann @jacobheun Q: How are we going to store automated policy per Pinning service?
IIUC we will have the default which is manual (null
and/or explicit manual
?) and there will be opt-in mfs-root
policy, which enables remote pin on every MFS root change.
Do we need to add an array PinningService.Policies
?
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.
@aschmahmann @petar
I believe we need array. For now it will be either "manual" or "all in MFS", but I can imagine the future when one can have both MFS and local pins mirrored to remote service:
Policies: ["mfs-root", "local-pins"]
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.
Yep, I think there should be an array of policies per service. I think @petar wasn't sure if we wanted Pinning.RemoteServices to be an array or a map indexed by the service name.
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.
Based on my chat with @aschmahmann on Friday, we want something like #113 (review)
Some details inline.
Co-authored-by: Marcin Rataj <[email protected]>
I'm closing this to decrease noise and confusion. |
@lidel this is the basic config struct. We can finalize when we know that this actually what we need.