fix: setup cache on tmp dir#17
Conversation
Signed-off-by: Gustavo Carvalho <gustavo.carvalho@container-solutions.com>
📝 WalkthroughWalkthroughThe PR adds an internal helper function ChangesCustodian Ephemeral Cache Path
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@main.go`:
- Around line 375-377: custodianCachePath currently returns a global temp file
which causes cross-run contention; change it to produce a per-run cache file
under the run's output directory (use req.OutputDir) instead of os.TempDir().
For example, modify custodianCachePath to accept an outputDir parameter (or
remove it and inline a new join using req.OutputDir where called) and return
filepath.Join(outputDir, "cloud-custodian.cache"); update all call sites (the
locations that call custodianCachePath() around the uses at/near
custodianCachePath and the caller at line ~507) to pass req.OutputDir so each
run gets an isolated cache file that is automatically within the run's output
directory.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| func custodianCachePath() string { | ||
| return filepath.Join(os.TempDir(), "cloud-custodian.cache") | ||
| } |
There was a problem hiding this comment.
Use a run-scoped cache path instead of a shared global temp file.
Line 375 and Line 507 currently force every execution to the same cache file (/tmp/cloud-custodian.cache or $TMPDIR/...). That introduces cross-run cache sharing and potential contention/stale data between checks or concurrent plugin executions. Prefer a per-execution cache file under req.OutputDir so isolation and cleanup are automatic.
Suggested diff
-func custodianCachePath() string {
- return filepath.Join(os.TempDir(), "cloud-custodian.cache")
+func custodianCachePath(outputDir string) string {
+ return filepath.Join(outputDir, "cloud-custodian.cache")
}
@@
- args = append(args, "--dryrun", "-s", req.OutputDir, "--cache", custodianCachePath(), policyPath)
+ args = append(args, "--dryrun", "-s", req.OutputDir, "--cache", custodianCachePath(req.OutputDir), policyPath)Also applies to: 507-507
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@main.go` around lines 375 - 377, custodianCachePath currently returns a
global temp file which causes cross-run contention; change it to produce a
per-run cache file under the run's output directory (use req.OutputDir) instead
of os.TempDir(). For example, modify custodianCachePath to accept an outputDir
parameter (or remove it and inline a new join using req.OutputDir where called)
and return filepath.Join(outputDir, "cloud-custodian.cache"); update all call
sites (the locations that call custodianCachePath() around the uses at/near
custodianCachePath and the caller at line ~507) to pass req.OutputDir so each
run gets an isolated cache file that is automatically within the run's output
directory.
Summary by CodeRabbit