Skip to content
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

feat: support symlink resolution to one level #1708

Conversation

michaelschuett-tomtom
Copy link

Changes

This supports symlink files to be uploaded provided it can resolve the link. For the golang API this means the link points directly do regular file. If the symlink points to another symlink it will fail and databricks will act exactly as it does today and ignore the file.

This is largely because I want to write a bazel rule that uses databricks and without this support it's close to impossible as bazel heavily uses symlinks.

Tests

  1. Created a symlink and ran databrick bundle deploy and see it's uploaded to the files folder.
  2. Create a symlink that points to another symlink and see that no error is thrown.
  3. Create symlinks that reference each other and see that no error is thrown.


seen[name] = struct{}{}
out = append(out, NewFile(w.root, fs.FileInfoToDirEntry(fileInfo), name))

Choose a reason for hiding this comment

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

I think the only open question provided this is accepted is if we should ever throw errors around symlinks since this will be a breaking change. Not returning any errors but supporting them would not be a breaking change.

@pietern
Copy link
Contributor

pietern commented Sep 6, 2024

Thanks for posting the PR.

I commented in #1706 (comment) with the rationale for not resolving symlinks. Let's continue the thread there.

@pietern
Copy link
Contributor

pietern commented Sep 23, 2024

In the linked comment I propose using rsync -L to build a symlink-free copy of the tree prior to calling bundle commands.

Adding proper symlink support introduces quite a few difficult corner cases I'd rather not have to deal with.

I'm closing this PR, but we can revisit it if there is more evidence that suggests the benefits outweigh the costs.

@pietern pietern closed this Sep 23, 2024
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.

2 participants