Skip to content

Handle broken symlinks in devcontainer set up#5250

Merged
ViniciustCosta merged 2 commits intomasterfrom
dylanj/symlink
May 5, 2026
Merged

Handle broken symlinks in devcontainer set up#5250
ViniciustCosta merged 2 commits intomasterfrom
dylanj/symlink

Conversation

@dylanjew
Copy link
Copy Markdown
Collaborator

@dylanjew dylanjew commented Apr 21, 2026

I was running into an issue running the server in the devcontainer when I had initialized the config on my host machine.

This PR checks and cleans up broken symlinks correctly

After this change I can run the server in the devcontainer and the symlink is fixed

@dylanjew dylanjew requested a review from a team as a code owner April 21, 2026 17:35
@dylanjew dylanjew requested a review from aakallam April 21, 2026 17:36
Comment thread src/local/butler/common.py Outdated
os.symlink(src, target)

assert os.path.exists(target), f'Failed to create {target} symlink for {src}.'
assert os.path.lexists(target), f'Failed to create {target} symlink for {src}.'
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I get changing to lexists for the cleanup method but shouldn't we catch here if the symlink is broken?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

makes sense! done

@dylanjew dylanjew force-pushed the dylanj/symlink branch 6 times, most recently from 4476c20 to fc10005 Compare April 30, 2026 15:04
@dylanjew dylanjew requested a review from ViniciustCosta April 30, 2026 15:04
Comment thread src/local/butler/common.py
@ViniciustCosta ViniciustCosta merged commit cd92c85 into master May 5, 2026
10 of 11 checks passed
@ViniciustCosta ViniciustCosta deleted the dylanj/symlink branch May 5, 2026 16:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants