New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use time.Duration (10m) for maxAge in file cache #5438

Closed
bep opened this Issue Nov 13, 2018 · 6 comments

Comments

Projects
None yet
3 participants
@bep
Member

bep commented Nov 13, 2018

The current:

[caches]
[caches.getjson]
dir = ":cacheDir"
maxAge = -1
[caches.getcsv]
dir = ":cacheDir"
maxAge = -1
[caches.images]
dir = ":resourceDir/_gen"
maxAge = -1
[caches.assets]
dir = ":resourceDir/_gen"
maxAge = -1

-1 means forever. I guess we should establish a good baseline for this before it's too late ...

Note that the motivation for cache expire in the above is different:

  • getJSON / getCSV, handle potential changes in the source (and potentially also clean up unused)
  • images/assets: Clean up unused files

The current value is noted in seconds, but I would be happy to change that to something bigger if that makes sense.

Suggest value(s) for the above?

@moorereason @regisphilibert @onedrawingperday and gang

@bep bep changed the title from Establis a better default set of maxAge for file caches to Establish a better default set of maxAge for file caches Nov 13, 2018

@bep

This comment has been minimized.

Member

bep commented Nov 13, 2018

Never mind, the "cleanup logic" will not work without some additional logic, so close this.

@bep bep closed this Nov 13, 2018

@regisphilibert

This comment has been minimized.

regisphilibert commented Nov 13, 2018

Suggest value(s) for the above?

The current value is noted in seconds, but I would be happy to change that to something bigger if that makes sense.

Is there another issue where this conversation could be continued?

@bep

This comment has been minimized.

Member

bep commented Nov 13, 2018

Is there another issue where this conversation could be continued?

Not that I know about. Thinking about it, seconds is kind of easy to reason about in unit tests (at least faster). A little harder to express in years ... 31536000. But it's not hard.

@bep

This comment has been minimized.

Member

bep commented Nov 13, 2018

Reopen this in relation to #5439 -- I'm going to move the "cleaning routine" to the new file cache (hugo --gc) and with that the defaults above will have more meaning, even for the JSON/CSV case.

@bep bep reopened this Nov 13, 2018

@moorereason

This comment has been minimized.

Contributor

moorereason commented Nov 14, 2018

Having dealt with DNS TTLs a ton over the years, I much prefer time.Duration for config options instead of using seconds.

maxAge = "10m"

time.Duration is a int64, so -1 would be supported.

fmt.Printf("%v\n", time.Duration(-1)) // Prints -1ns

if time.Duration(-1) < 0 {
    // this works
}

if time.Now().Sub(timestamp) > maxAge {
    // something like this
}
@regisphilibert

This comment has been minimized.

regisphilibert commented Nov 14, 2018

☝️ Yes, seconds can be made readable with inline compute (10 * 24 *60 * 60), but as it won't be available in Hugo config files, I agree with @moorereason for something both easily readable and writable like 10m, 12h, 5d etc...

Suggest value(s) for the above?

getJSON and getCSV:
I'd suggest keeping it that way (-1) so current setups are not impacted.

@bep bep changed the title from Establish a better default set of maxAge for file caches to Use time.Durarion (10m) for maxAge in file cache Nov 14, 2018

@bep bep changed the title from Use time.Durarion (10m) for maxAge in file cache to Use time.Duration (10m) for maxAge in file cache Nov 14, 2018

bep added a commit to bep/hugo that referenced this issue Nov 14, 2018

@bep bep closed this in d3489eb Nov 14, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment