[Resolves #1268] Modify ConfigFileNotFound Error to Conditionally Include Valid Stack Paths

[X-Guardian]

Modifies the Sceptre Plan ConfigFileNotFound error to only include the list of valid stack paths from the command path specified and only there are less than 10 stacks.

Example

sceptre launch dev/service1/foo.yaml

New Error Message (when there are less than 10 stacks under the specified parent path)

"No stacks detected from the given path 'dev/service1/foo.yaml'. Valid stack paths under 'dev/service1' are: ['dev/service1/foo1.yaml', 'dev/service1/foo2.yaml', 'dev/service1/subservice1/foo3.yaml']"

New Error Message (when there are 10 stacks or more under the specified parent path)

"No stacks detected from the given path 'dev/service1/foo.yaml'."

• Resolves When a non-existent Stack is Launched, the List of Valid Stack Paths is not useful in a Large Workspace  #1268

PR Checklist

• Wrote a good commit message & description [see guide below].
• Commit message starts with [Resolve #issue-number].
• Added/Updated unit tests.
• Added/Updated integration tests (if applicable).
• All unit tests (make test) are passing.
• Used the same coding conventions as the rest of the project.
• The new code passes pre-commit validations (pre-commit run --all-files).
• The PR relates to only one subject with a clear title.
  and description in grammatically correct, complete sentences.

Approver/Reviewer Checklist

• Before merge squash related commits.

Other Information

Guide to writing a good commit

[jfalkenstein]

A few thoughts on this:

• This seems rather deeply nested. I'd like to avoid getting so nested, especially in this function. It hurts readability. I also don't think we need to do it this way.
• Rather than crawling every combination of segments and comparing parents, I don't think we need to get that intense about it. Rather, I think a simpler and more straight-forward logic here would be to do this:
  1. Check if there's a "/" in the command path. If not, then the prefix to use is '' (all strings technically start with ''). If there IS, do prefix, _ = command_path.rsplit('/' 1) to get the prefix.
  2. Filter the valid paths for those that start with the prefix. You can either use filter() or a list comprehension.

Done this way, it would look more like:

Suggested change

	command_path = pathlib.PurePath(self.context.command_path)
	filtered_valid_stack_paths = []
	for stack_path in self._valid_stack_paths():
	for parent in pathlib.PurePath(stack_path).parents:
	if command_path.parent == parent:
	filtered_valid_stack_paths.append(stack_path)
	break
	if filtered_valid_stack_paths and len(filtered_valid_stack_paths) > MAX_VALID_STACK_PATH_COUNT:
	filtered_valid_stack_paths = None
	break
	if '/' in self.context.command_path:
	prefix, _ = self.context.command_path.rsplit('/', 1)
	else:
	prefix = ''
	all_paths = self._valid_stack_paths()
	# We'll fall back to all_paths if no valid path starts with the prefix
	filtered_valid_stack_paths = [p for p in all_paths if p.startswith(prefix + '/')] or all_paths
	if len(filtered_valid_stack_paths) > MAX_VALID_STACK_PATH_COUNT:
	filtered_valid_stack_paths = []

[X-Guardian]

Refactored function to use this suggestion with pathlib rather than / processing.

[jfalkenstein]

I'd also like to see a test around this functionality.

[jfalkenstein]

@X-Guardian, what are the status on the tests for this PR?

[alex-harvey-z3q]

@X-Guardian Do you have any plans to finish this PR?

[X-Guardian]

From what I remember it works, but I don't use Sceptre any more, so won't be writing any tests, which is what I think was blocking this getting merged.

[alex-harvey-z3q]

@zaro0508 I have had a look at this stale PR and I agree with @X-Guardian and I think the PR should be merged as-is. The new private method is simply tweaking the error message seen and I do not think that such a trivial change justifies us adding more unit test code. Nor is it obvious to me how you would test this anyway.

Could we merge? I've added it into my integration branch and I'm certainly happy with it.

Per above