Utils for downloading and syncing fw-headless projects locally #2066
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughThe PR adds infrastructure for downloading and syncing fw-headless projects. Changes include adding Moq mock support to LcmDebugger, implementing a SyncDownloadedProject extension method for end-to-end sync operations, creating a Node.js script to orchestrate project downloads from Kubernetes pods via tar.gz, and adding corresponding deployment configurations. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes The changes span configuration (trivial), simple DI/mocking setup (straightforward), and a complex Node.js orchestration script with Kubernetes interaction, tar processing, HTTP server management, and async flow control requiring careful logic verification. Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
bot
added
💻 FW Lite
|
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (13)
deployment/download-fw-headless-project.js (8)
103-116:fetchrequires Node 18+; usehttpHEAD or guard.To avoid Node version issues, use
http.request({method:"HEAD"})or dynamically import a polyfill. Also add a timeout.
90-98: Possible port conflict on 8088.Taskfile also uses 8088 for hgweb forwarding. Pick a free port dynamically or allow override via CLI/env.
81-85: Python dependency in container is brittle.Many images lack python3. Prefer
kubectl cpfor the tar or streamkubectl exec ... tar czf -to local. If keeping Python, add a preflight check and a BusyBox httpd fallback.
45-47: Command building lacks escaping.
runKubectl(args.join(" "))risks shell parsing issues if args contain spaces/specials. UsespawnSync("kubectl", args, ...)or escape args.
32-36: Align downloads path with repo structure.Current path is CWD‑relative. Resolve relative to this script so artifacts land in deployment/_downloads (which is ignored).
-import path from "path"; +import path from "path"; +import { fileURLToPath } from "url"; @@ -const localExtractDir = path.resolve(`_downloads/${projectCode}-${timestamp}`); +const __dirname = path.dirname(fileURLToPath(import.meta.url)); +const localExtractDir = path.resolve(__dirname, `_downloads/${projectCode}-${timestamp}`);
12-13: Remove unused variables.
pipeandlocalTararen’t used. Also the timestamp comment shows an underscore not present in the computed value.Also applies to: 30-33
49-61: Pod selection robustness.
items[0]may pick a non‑Running pod in rolling updates. Filter for Running and consider newest.
215-223: More defensive cleanup.Guard against missing pid file and ignore errors; you already do that. Also kill
pfProcesswith SIGINT and wait for exit to avoid orphaned forwards.backend/FwLite/LcmDebugger/LcmDebugger.csproj (1)
13-13: Pin Moq version or use central package management.Unpinned packages can cause nondeterministic builds. If you’re not using Directory.Packages.props, add an explicit version. If you are, mark this with
Versionthere and considerPrivateAssets="all"so Moq doesn’t flow transitively.Example:
<PackageReference Include="Moq" Version="4.20.72" PrivateAssets="all" />backend/FwLite/LcmDebugger/Program.cs (2)
19-19: Make DI registration explicit and strict.Register the service type explicitly and use
MockBehavior.Strictto fail fast if it’s accidentally used.-builder.Services.AddScoped((_services) => new Mock<IServerHttpClientProvider>().Object); +builder.Services.AddScoped<IServerHttpClientProvider>(_ => + new Mock<IServerHttpClientProvider>(MockBehavior.Strict).Object);
26-27: Avoid commented code in main; gate under DEBUG or move to a sample.Keeps Program clean and avoids drift.
backend/FwLite/LcmDebugger/Utils.cs (2)
49-50: Validate that crdt.sqlite exists before attempting to open.If the file doesn't exist, the subsequent
OpenProjectcall will fail with a less helpful error message.Add a file existence check:
var crdtDbPath = Path.Combine(currProjRoot, "crdt.sqlite"); + if (!File.Exists(crdtDbPath)) + throw new InvalidOperationException($"CRDT database not found at: {crdtDbPath}"); var crdtProject = new CrdtProject("unused-project-code", crdtDbPath);
51-51: Consider using a safe cast pattern.The direct cast to
CrdtMiniLcmApiwill throwInvalidCastExceptionif the implementation type changes. WhileOpenProjectcurrently returns the correct type, a safer pattern would verify the type or use pattern matching.Use pattern matching or an explicit type check:
- var crdtMiniLcmApi = (CrdtMiniLcmApi)await services.GetRequiredService<CrdtProjectsService>().OpenProject(crdtProject, services); + var api = await services.GetRequiredService<CrdtProjectsService>().OpenProject(crdtProject, services); + if (api is not CrdtMiniLcmApi crdtMiniLcmApi) + throw new InvalidOperationException($"Expected CrdtMiniLcmApi but got {api.GetType().Name}"); Console.WriteLine($"Crdt Project: {crdtMiniLcmApi.ProjectData.Code}");
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
backend/FwLite/LcmDebugger/LcmDebugger.csproj(1 hunks)backend/FwLite/LcmDebugger/Program.cs(1 hunks)backend/FwLite/LcmDebugger/Utils.cs(2 hunks)deployment/.gitignore(1 hunks)deployment/Taskfile.yml(1 hunks)deployment/download-fw-headless-project.js(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
backend/FwLite/LcmDebugger/Utils.cs (3)
backend/FwLite/LcmCrdt/CrdtMiniLcmApi.cs (1)
CrdtMiniLcmApi(24-809)backend/FwLite/LcmCrdt/CrdtProjectsService.cs (1)
CrdtProjectsService(16-406)backend/FwLite/FwLiteProjectSync/CrdtFwdataProjectSyncService.cs (1)
CrdtFwdataProjectSyncService(16-164)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Analyze (csharp)
- GitHub Check: frontend-component-unit-tests
- GitHub Check: Build FW Lite and run tests
- GitHub Check: frontend
🔇 Additional comments (4)
deployment/.gitignore (1)
1-1: LGTM, but verify CWD assumptions.This ignores deployment/_downloads. Ensure the download script writes to deployment/_downloads (not repo root), otherwise artifacts may be untracked. Consider resolving the downloads path relative to the script file.
deployment/Taskfile.yml (1)
105-118: Task wiring looks good; confirm working directory.Command runs
node download-fw-headless-project.js ...without an explicitdir. Taskfile lives in deployment/, so this should execute there. If contributors run Task from repo root, confirm Task discovers this Taskfile or documenttask -d deployment ....backend/FwLite/LcmDebugger/Utils.cs (2)
1-1: LGTM!The new using directives are correctly added to support the CallerFilePath attribute and the new sync functionality.
Also applies to: 4-5
60-76: LGTM!The helper method correctly uses
CallerFilePathto locate the repository root and constructs the downloads path. The directory traversal logic properly handles edge cases (null parent directory) and provides clear error messages if the solution root cannot be found.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Both KUBECTL_REMOTE_COMMAND_WEBSOCKETS = false and --retries 10 are important and very effective.
myieye
force-pushed
the
script-for-downloading-fw-headless-projects
branch
from
fc8aa7d to
de1cc27
Compare
rmunn
left a comment
rmunn
left a comment
There was a problem hiding this comment.
Nice. Thank you for getting this done (and in Taskfile too); I've been wanting this for a while but never got around to implementing it.
This pull request adds robust support for downloading and syncing full FW Headless project folders from a Kubernetes pod for local debugging and integration. The changes include a new automation script and task for downloading project folders, updates to backend utilities to support syncing downloaded projects, and improvements to dependency management and ignore rules.
Deployment automation and backend integration:
Deployment scripts and tasks:
deployment/download-fw-headless-project.js, a Node.js script that automates finding the pod, tarring the project folder, serving it via a temporary Python HTTP server, port-forwarding, downloading, extracting locally, and cleaning up. This enables easy retrieval of entire project folders for local use.download-fw-headless-projectto invoke the script with simple arguments, streamlining the workflow for developers..gitignoreto exclude the_downloads/directory, preventing accidental commits of downloaded project data.Backend utility enhancements:
SyncDownloadedProjectmethod toUtils.cs, allowing backend code to open and sync a downloaded project folder by path, including logic to locate the default downloads directory relative to the repo root.Program.csto register a mockIServerHttpClientProviderfor improved testability and added example usage of the new sync method.And here's a bookmarklet that when used on a project page in Lexbox will put a cli command on your clipboard for downloading the current project: