netlify-plugin-cache icon indicating copy to clipboard operation
netlify-plugin-cache copied to clipboard

Change the default value of the `paths` input

Open ehmicky opened this issue 4 years ago • 10 comments

Netlify caches ~/.cache by default. Having it as the default value for the paths input would cause this directory (which is often big) to be cached twice.

ehmicky avatar Jun 08 '20 19:06 ehmicky

Whoops, great point. Any ideas for a better default, or would it make more sense to stick with required: true and wait on adding this to the directory until the UI allows user input (if that's on the roadmap)?

I went back and forth on this and I'm worried I'm "cheating" the system as it is now by setting any default, since 95% of users will probably want/need to add this to their netlify.toml to be truly useful. But open to suggestions either way!

jakejarvis avatar Jun 16 '20 13:06 jakejarvis

I can confirm we are planning to provide a way to set inputs through the UI :+1:

It looks to me that, since this plugin is generic, most users might want to specify their own file path. Since the only way to do it at the moment is through netlify.toml, it looks like this might be the preferred way at the moment?

From that perspective, providing a 0-default configuration is hard in this case. Do you think it might make case to make the default value of paths an empty array? Like this, once UI inputs become available, it will be possible to install this plugin through the UI, then afterwards add some inputs.

What do you think?

ehmicky avatar Jun 16 '20 16:06 ehmicky

Hey @ehmicky. You said that netlify caches .cache by default. I just had a look at the run-build-function.sh and couldn't find this. I only see that .cache/pip is referenced? Or am I messing something up?

apriljunge avatar Jan 15 '22 13:01 apriljunge

Caching .cache has been reverted since the last comment by https://github.com/netlify/build-image/pull/443

That being said, I am still wondering whether making users chose which paths to cache is better (since it is more flexible and avoid caching too much, which ends up slowing builds) than providing .cache as default? What are your thoughts on this?

ehmicky avatar Jan 17 '22 15:01 ehmicky

Thanks for clarifying. I agree with your proposal. Empty array would be the best default value. Others might be misleading and cause unwanted errors. Also, there could be the possibility to add an environment variable, so the plugin can be configured completely via GUI.

Do you know what would happen if someone would still manually cache something that is already automatically cached by netlify like .cache/pip? Would this cause errors or duplicated files?

apriljunge avatar Jan 17 '22 20:01 apriljunge

But how do you deal with backwards compatibility? Since plugins are not pinned with versions, in some installations files might not be cached anymore.

apriljunge avatar Jan 17 '22 20:01 apriljunge

Build plugins versions can now be pinned, we've added the feature recently (documentation) :)

Do you know what would happen if someone would still manually cache something that is already automatically cached by netlify like .cache/pip? Would this cause errors or duplicated files?

Netlify hashes the contents of files being cached, then caches that hash together with the files. When trying to cache twice the same file, if the hash matches, we skip it (link to code).

Unfortunately, we only do this for files cached by plugins with utils.cache. We cache most files automatically as part of our build process in the build-image (like .cache/pip). For those, the previously cached contents will be deleted, and the cache will be copied again. So this would end up being a noop, but might make the build last a few more seconds, depending on the size of the directory to cache. We plan on refactoring the build-image, which would fix this problem, but this is not in our immediate roadmap at the moment unfortunately.

ehmicky avatar Jan 17 '22 20:01 ehmicky

build plugins versions can now be pinned, we've added the feature recently

Really nice :+1:! When I previously installed this plugin via netlify.toml and didnt specify a version in package.json, it would still be a problem right? I just saw that the docs say "For a plugin published to npm, you also add it as a dependency.". I can only speak for myself, but I always missed this part and only referenced it in netlify.toml. Would netlify lock a specific version automatically in this case?

We cache most files automatically as part of our build process in the build-image (like .cache/pip). For those, the previously cached contents will be deleted, and the cache will be copied again.

Thank you for clarification. I think thats totally okay. Maybe a small notice in the readme with a reference about this would be great. I'll do a PR for this in the near future.

apriljunge avatar Jan 19 '22 07:01 apriljunge

Would netlify lock a specific version automatically in this case?

It will then behave roughly the same way as when a plugin is installed through the UI, including the version being pinned. Please note that we now recommend installing a plugin either in the UI, or in package.json, and have undocumented that third way of installing a plugin (in netlify.toml but not in package.json).

ehmicky avatar Jan 19 '22 14:01 ehmicky

Very nice! Thank you. In summary, there is nothing to be said against changing the default to empty array.

@jakejarvis can you change the default as proposed? I can create a PR, but I'm not sure if this really helps as most of the work is publishing the new version.

apriljunge avatar Jan 19 '22 16:01 apriljunge