Conversation
📝 WalkthroughWalkthroughThese changes update the development container configuration to support a full-stack monorepo setup with separate backend (Python venv) and frontend (Node.js/pnpm) workspace directories, and adds Git LFS support for the container environment. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
|
probably want to merge #115 and then update this branch before you start using this in instantiated templates...I'm not sure exactly what will happen otherwise |
There was a problem hiding this comment.
are you running your devcontainers inside WSL? maybe that explains some of the differences in behaviors we've observed. my files are on my regular windows hard drive. I know it's better to have them in the WSL "hard drive", but I haven't had a chance to figure that out yet
There was a problem hiding this comment.
yes using WSL "hard drive"
| "service": "devcontainer", | ||
| "workspaceFolder": "/workspaces/${localWorkspaceFolderBasename}", | ||
| "features": {{% endraw %}{% if is_child_of_copier_base_template is not defined %}{% raw %} | ||
| "ghcr.io/devcontainers/features/git-lfs:1.2.5": { |
There was a problem hiding this comment.
should this be a change in the base template?
There was a problem hiding this comment.
if you wanna just add a note to yourself, and not make the change now, that's fine. I just don't want to forget
There was a problem hiding this comment.
probably worthwhile to put at that level too. Since figuring it out i haven't gone back and tried to use other non "python/nuxt project" based things as a devcontainer. So i wouldn't be surprised if its broken there.
There was a problem hiding this comment.
There was a problem hiding this comment.
merged and updated this branch. :beg: for another approval please
| ENV VENV_PATH=/workspaces/${REPO_NAME}/backend/.venv | ||
| ENV PNPM_STORE=/workspaces/${REPO_NAME}/.pnpm-store | ||
| ENV FRONTEND_NODE_MODULES=/workspaces/${REPO_NAME}/frontend/node_modules |
There was a problem hiding this comment.
we have several copier templates that have venv/node_modules that are not in the repo root. at some point (not saying now) we should probably move this into copier-base template and make a copier task in the sub-template that just adjusts the paths. the task could be defined in the copier.yaml.jinja-base in the base template, and just read from the envs.json that defines the paths for the different python/node environments in the template
There was a problem hiding this comment.
Created this fwiw LabAutomationAndScreening/copier-base-template#127
## Link to Issue or Message thread See LabAutomationAndScreening/copier-nuxt-python-intranet-app#117 ## Why is this change necessary? Propogating the devcontainers fix for failures to init git-lfs hook when we already have pre-commit in place. This impacts both devcontainers and codespaces. ## How does this change address the issue? Explicitly adds the git-lfs feature (which is a transitive already) and sets the "autoPull" property to false. This makes it so the hook wont be installed. If we ever end up needing git-lfs we can figure out how to make this and pre-commit play nicely together. > [!NOTE] > As stated in the other PR i did try to fiddle the pre-commit install order and it did not help it kept failing out. ## What side effects does this change have? Not git hook attempted to be setup for git-lfs so the container then fully initializes since there are no failures. ## How is this change tested? Locally spinning up devcontainer as well as using using it previously with the fix in the python/nuxt template. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Updated development container configuration to include Git LFS with auto-pull disabled to prevent pre-commit hook interference during development. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
2 participants
Why is this change necessary?
Devcontainer and technically the codespaces have been failing during creation.
Codespaces just fails, shows error and moves on. Devcontainers have more issues and in some cases will not build at all and generally fail with one of 2 errors below.
git-lfs details
When git-lfs tries to install its hook (added in v.1.2.5 of the feature) since pre-commit is already there. This is the "soft failure" where it reports but does not cause it to stop anything in codespaces. But in devcontainers it does fail and stops any future setup from this point on.
Note
I did try to reorder where pre-commit install was called but that didn't seem to help. This seemed like a reasonable solution since we are not using git-lfs today and if we do at some point we can dig in then.
volume mount issues
Below you can see that these fail when spinning up a brand new devcontainer. Pretty sure this only impacts local devcontainers since codespaces likely are not using the volume mounts in the same way. This is a hard failure for local devcontainers.
How does this change address the issue?
For all docker mounts we are now pre-creating the dirs and updating the perms. And for git-lfs feature we have just disabled the install step for the git hook.
What side effects does this change have?
Working devcontainer build
How is this change tested?
Locally using sensor-monitor
Summary by CodeRabbit
Release Notes