Conversation
isc-dchui
requested review from
isc-cborbonm,
isc-egabhart,
isc-eneil,
isc-jili,
isc-jlechtne,
isc-kiyer,
isc-pbarton and
isc-tleavitt
as code owners
isc-jili
left a comment
isc-jili
left a comment
There was a problem hiding this comment.
@isc-dchui This looks really good! I made a pass and few small comments
| set horologTime = ##class(%Library.File).GetFileDateModified(filePath) | ||
| set currentMTime = $zdatetime(horologTime, 3) | ||
| do defObj.RefreshCacheEntry(cacheObj, filePath, currentMTime) | ||
| $$$ThrowOnError(status) |
There was a problem hiding this comment.
I'm assuming the status is supposed to come from RefreshCacheEntry(). How is status being tracked though, since it's not getting passed to the caller of RefreshCacheEntry() here?
There was a problem hiding this comment.
Ah good catch. This is a remnant of slightly different error handling. I've also standardized the method to just set the Output arg and quit throughout.
| // Compare cached mtime with current mtime | ||
| if (cacheObj.FileMTime '= currentMTime) { | ||
| do defObj.RefreshCacheEntry(cacheObj, filePath, currentMTime) | ||
| if $$$ISERR(status) { |
There was a problem hiding this comment.
Same question as above for how is status being tracked for this call of RefreshCacheEntry()?
There was a problem hiding this comment.
Removed that remnant too.
src/cls/IPM/Main.cls
Outdated
| set server = ##class(%IPM.Repo.Definition).ServerDefinitionKeyOpen(pRepoName,,.tSC) | ||
| $$$ThrowOnError(tSC) | ||
| if server.%IsA("%IPM.Repo.Filesystem.Definition") { | ||
| write !,"Last full cache rebuild for '", pRepoName, "' was on ", server.CacheLastRebuilt, " (UTC). Run 'repo -n "_pRepoName_" -rebuild-cache' to refresh the cache.",! |
There was a problem hiding this comment.
I find the wording here a bit confusing. Is "Run 'repo -n "pRepoName" -rebuild-cache' to refresh the cache" supposed to be just informative? Or is it asking the user to do that as an action item before proceeding further?
ie. Could potentially confuse users if all they're interested in is seeing which modules are available
There was a problem hiding this comment.
Just informative. I've modified it to hopefully indicate that better.
2 participants
Fixes #536
Improves the filesystem repo caching by proactively rebuilding cache and improving cache building performance by about an order of magnitude. Specifically:
-rebuild-cacheflag torepocommand to manually fully rebuild (with purge) cache.