tk tool importers: Handle deleted files
#872
Merged
Conversation
This file contains 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
|
julienduchesne
force-pushed
the
julienduchesne/tool-importers-deleted-files
branch
2 times, most recently
from
8c33773 to
20bac77
Compare
One of the issues we are facing with this command is whenever someone deletes a file and it's still being used somewhere: - We can't pass this file to the `importers` command because it will fail trying to find symlinks - Jsonnet main files which are using this file will not be found - We end up with erroring projects that are only found out the next time they are evaluated This PR adds support for deleted files via a `deleted:` prefix that can be added to files that were deleted. In that case, a simple filepath.Abs logic is used instead of trying to find symlinks and the same import resolution process happens as with existing files Ex: ```console tk tool importers deleted:lib/external-secrets/main.libsonnet ```
julienduchesne
force-pushed
the
julienduchesne/tool-importers-deleted-files
branch
from
20bac77 to
dcf9018
Compare
jvrplmlmn
reviewed
Jun 29, 2023
| @@ -154,6 +208,11 @@ func TestFindImportersForFiles(t *testing.T) { | |||
| require.NoError(t, err) | |||
| require.NotEmpty(t, files) | |||
| for _, file := range files { | |||
| // This project is known to be invalid (as the name suggests) | |||
| if strings.Contains(file, "using-deleted-stuff") { | |||
There was a problem hiding this comment.
Should this be part of findImportersTestCase? That way you can have next to the test information if you should skip here or rather, expect an error or not.
There was a problem hiding this comment.
hmm, I don't think so. It's a sanity check. The command to find importers does not eval jsonnet (because that would be too expensive), and for that that reason, it does not necessarily throw errors on invalid jsonnet (it's not its purpose). So this eval is there to check that the test cases are actually valid
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.
One of the issues we are facing with this command is whenever someone deletes a file and it's still being used somewhere:
importerscommand because it will fail trying to find symlinksThis PR adds support for deleted files via a
deleted:prefix that can be added to files that were deleted. In that case, a simple filepath.Abs logic is used instead of trying to find symlinks and the same import resolution process happens as with existing filesEx:
tk tool importers deleted:lib/external-secrets/main.libsonnet