Configurable cache file name #49
Conversation
Handy when you have configured two feeds pointing to the same packages folder.
|
|
|
@apeneve, sorry about the delay. Thanks for the contribution. I'm taking a look at this now! |
| @@ -22,6 +22,11 @@ | |||
| --> | |||
| <add key="packagesPath" value="" /> | |||
|
|
|||
| <!-- | |||
| Change the name of the internal cache file. Default is machine name (System.Environment.MachineName). | |||
There was a problem hiding this comment.
It's unclear whether putting a path here works (or even should work).
I would recommend updated the XML comment to state that this must be a file name and not a relative or absolute path. Additionally, I would validate at runtime that this is just a file name. In the future we could extend this to support relative paths but that seems unnecessary now and can be expanded without being a breaking change.
There was a problem hiding this comment.
Agreed. I updated the XML comment and added validation of a valid file name.
| } | ||
|
|
||
| if (fileName.EndsWith(suffix, StringComparison.OrdinalIgnoreCase)) | ||
| return fileName; |
There was a problem hiding this comment.
nitpick: curly { } around all if blocks, even if one line.
| readonly Func<string, bool, bool> _getSetting; | ||
| internal FuncSettingsProvider(Func<string,bool,bool> getSetting) | ||
| readonly Func<string, object, object> _getSetting; | ||
| internal FuncSettingsProvider(Func<string,object,object> getSetting) |
There was a problem hiding this comment.
nitpic: spaces between type parameters after the comma ,
|
Could you add some unit or integration test that verified this behavior? I'm thinking something that verifies that if the this setting is provided, the cache file appears in the packages directory ( |
…erverPackageCache
|
@joelverhagen I fixed up the nitpic comments and extended the XML comment a bit. I also added some tests to the ServerPackageRepository class to describe the different cache file name scenarios. |
|
@apeneve, wonderful! I'll work on merging this and shipping this as a |
|
@apeneve, this is available as 3.1.0 on nuget.org: |
|
Excellent! I will try it out right away.
|
Background for pull request: I have two nuget server sites sharing the same package repository folder. One public with authentication, and one internal. In this scenario I get file lock errors on the cache file, so I need to be able to configure different cache files for each site.