-
Notifications
You must be signed in to change notification settings - Fork 31
fix: Remove LocalGitConfigSource and replace with FileGitConfigSource #708
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
chrisburr
wants to merge
3
commits into
DIRACGrid:main
Choose a base branch
from
chrisburr:claude/mr-issue-706-01FxRPh3uyTxGeWSvdbLkHJQ
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
fix: Remove LocalGitConfigSource and replace with FileGitConfigSource #708
chrisburr
wants to merge
3
commits into
DIRACGrid:main
from
chrisburr:claude/mr-issue-706-01FxRPh3uyTxGeWSvdbLkHJQ
+49
−51
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add GitHub Actions workflow to automate version bumping and changelog generation using googleapis/release-please-action on main branch pushes.
This addresses issue DIRACGrid#706 by removing the problematic LocalGitConfigSource class which encountered file permission issues in containerized deployments. Changes: - Removed LocalGitConfigSource class that directly accessed local git repos - Created FileGitConfigSource as an alias to RemoteGitConfigSource for git+file:// URLs - FileGitConfigSource clones repos into temporary directories, avoiding permission issues - Removed _apply_default_scheme() that automatically defaulted to git+file:// - Updated CLI config validation to support both git+file:// and file:// schemes - Updated documentation to recommend git+https for production use The new approach uses git clone for both remote and local repositories, avoiding the security and permission problems with bind-mounted directories. Fixes DIRACGrid#706
for more information, see https://pre-commit.ci
Member
|
I can review this if you really want, but at a quick look it has a few "functionality breaking" bugs; the copilot version in #707 appears to be closer to working... Regards, |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #706
This PR removes the problematic
LocalGitConfigSourceclass which encountered file permission issues in containerized deployments when using bind-mounted read-only directories.Problem
The
LocalGitConfigSourceclass encountered multiple operational challenges in containerized deployments:Solution
LocalGitConfigSourceclass that directly accessed local git repositoriesFileGitConfigSourceas an alias toRemoteGitConfigSourceforgit+file://URLsFileGitConfigSourceclones repositories into temporary directories, avoiding permission issues_apply_default_scheme()that automatically defaulted togit+file://git+file://andfile://schemesgit+https://for production deploymentsBenefits
git+file://usageChanges
diracx-core/src/diracx/core/config/sources.py: Removed LocalGitConfigSource, added FileGitConfigSourcediracx-core/src/diracx/core/config/__init__.py: Removed LocalGitConfigSource from exportsdiracx-cli/src/diracx/cli/internal/config.py: Updated to support file:// schemedocs/admin/explanations/configuration.md: Updated documentationTesting
Existing tests using
git+file://URLs should continue to work with the newFileGitConfigSourceimplementation. Local development workflows usingrun_local.shremain unchanged.