fix deprecated file deletion

[pmeier]

What does this implement/fix?

The function

nebari/nebari/render.py

Lines 229 to 238 in 8bd56f9

	def inspect_files(
	source_dirs: str,
	output_dirs: str,
	source_base_dir: str,
	output_base_dir: str,
	ignore_filenames: List[str] = None,
	ignore_directories: List[str] = None,
	deleted_paths: List[str] = None,
	contents: Dict[str, str] = None,
	):

is called only once from

nebari/nebari/render.py

Lines 79 to 95 in 8bd56f9

	new, untracked, updated, deleted = inspect_files(
	source_dirs,
	output_dirs,
	source_base_dir=str(template_directory),
	output_base_dir=str(output_directory),
	ignore_filenames=[
	"terraform.tfstate",
	".terraform.lock.hcl",
	"terraform.tfstate.backup",
	],
	ignore_directories=[
	".terraform",
	"__pycache__",
	],
	deleted_paths=DEPRECATED_FILE_PATHS,
	contents=contents,
	)

As indicated by the variable name, deleted_paths=DEPRECATED_FILE_PATHS should schedule all deprecated files for deletion. However, inside inspect_files, deleted_paths is unconditionally overwritten:

nebari/nebari/render.py

Lines 275 to 279 in 8bd56f9

	deleted_paths = set()
	for path in deleted_paths:
	absolute_path = os.path.join(output_base_dir, path)
	if os.path.exists(absolute_path):
	deleted_paths.add(path)

Thus, the passed value of deleted_paths is completely ignored.

IIUC, the purpose of this block is to find all deprecated files that actually exist and return them so they can be deleted. This PR implements this.

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds a feature)
• Breaking change (fix or feature that would cause existing features not to work as expected)
• Documentation Update
• Code style update (formatting, renaming)
• Refactoring (no functional changes, no API changes)
• Build related changes
• Other (please describe):

Testing

• Did you test the pull request locally?
• Did you add new tests?

[iameskild]

/bot run tests

[iameskild]

/bot run tests

[github-actions]

Contributor Tests Triggered by @iameskild

• via comment
• Build is running here

[costrouc]

Wanted to write here just some history on this function on copying files and why I didn't choose a pre-built solution.

1. This code was written with the idea that we would be moving towards an extension system. Meaning that we have multiple src directories with one destination directory.
2. Not all "files" in src are coming from files. Some are computed e.g. the terraform *.tf.json files
3. There are cases to consider:
   • a file exists in multiple src directories... (decision: which one to choose?)
   • a file from a src doesn't exist in destination (decision: easy decision just copy over)
   • a file from a src is different in destination (decision: easy copy over since src is source of truth but use should be notified of change)
   • a file in destination exists that isn't in any src directories (decision: keep? but notify user there is an unwatched file?)

Basically the multiple src issue and dynamic generation of files is what led me to write those functions.