Skip to content
This repository was archived by the owner on Jun 19, 2023. It is now read-only.

feat: add namesys publish options #94

Merged
merged 3 commits into from
Jan 24, 2023
Merged

feat: add namesys publish options #94

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8e5d734
feat: add namesys publish options
hacdias Dec 7, 2022
428e597
feat: export DefaultIPNSRecordEOL
hacdias Jan 24, 2023
a5b18d4
feat: export DefaultIPNSRecordTTL
hacdias Jan 24, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 45 additions & 0 deletions options/namesys/opts.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,3 +72,48 @@ func ProcessOpts(opts []ResolveOpt) ResolveOpts {
}
return rsopts
}

// defaultRecordEOL specifies the time that the network will cache IPNS
// records after being published. Records should be re-published before this
// interval expires. We use the same default expiration as the DHT.
const defaultRecordEOL = 48 * time.Hour

// PublishOptions specifies options for publishing an IPNS record.
type PublishOptions struct {
EOL time.Time
TTL time.Duration
}

// DefaultPublishOptions returns the default options for publishing an IPNS record.
func DefaultPublishOptions() PublishOptions {
return PublishOptions{
EOL: time.Now().Add(defaultRecordEOL),
TTL: time.Minute,
Copy link
Member

@lidel lidel Jan 24, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This default should be moved to a const too (DefaultIPNSRecordTTL).

💭 I wonder if 1 minute is a good default. We are caching IPNS lookup results for 1m in Kubo, so I understand setting TTL to 1m keeps the current behavior, but that is just bare minimum for performance reasons.

For context, when you add DNS record at many registrars, the default TTL for DNS records is either 12 hours or 1 hour, and is lowered manually by the user to 1 minute only for websites which need fast updates.

If we keep this low from the start, the performance improvement of TTL will be close to zero, because nobody will adjust it to a bigger value (tyranny of the default).

@hacdias I don't want to block this PR on the decision, let's go with 1m for now, but mind opening a PR against https://github.com/ipfs/specs/blob/main/ipns/IPNS.md#ipns-record that adds suggested default to be 12h, so we can then gather feedback?

Copy link
Member Author

@hacdias hacdias Jan 24, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will add that to my ToDos (today or tomorrow). Constant exported.

Copy link
Member Author

@hacdias hacdias Jan 25, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ipfs/specs#371

}
}

// PublishOption is used to set an option for PublishOpts.
type PublishOption func(*PublishOptions)

// PublishWithEOL sets an EOL.
func PublishWithEOL(eol time.Time) PublishOption {
return func(o *PublishOptions) {
o.EOL = eol
}
}

// PublishWithEOL sets a TTL.
func PublishWithTTL(ttl time.Duration) PublishOption {
return func(o *PublishOptions) {
o.TTL = ttl
}
}

// ProcessPublishOptions converts an array of PublishOpt into a PublishOpts object.
func ProcessPublishOptions(opts []PublishOption) PublishOptions {
rsopts := DefaultPublishOptions()
for _, option := range opts {
option(&rsopts)
}
return rsopts
}