Skip to content

feat: Add in support for nested stack's code sync with Api's and StepFunctions#3948

Merged
mndeveci merged 14 commits intoaws:developfrom
qingchm:other_resource_nested
Jun 10, 2022
Merged

feat: Add in support for nested stack's code sync with Api's and StepFunctions#3948
mndeveci merged 14 commits intoaws:developfrom
qingchm:other_resource_nested

Conversation

@qingchm
Copy link
Contributor

@qingchm qingchm commented Jun 8, 2022

Which issue(s) does this change fix?

N.A.

Why is this change necessary?

Since currently we are implementing code sync support for nested stack, this will make Api and StepFunction resources within child stacks also syncable through sam sync --code.

How does it address the issue?

It resolves the definition files from path relative to child template, to path relative to root template. For base_dir use cases, this will still be base_dir based.

What side effects does this change have?

Not known.

Mandatory Checklist

PRs will only be reviewed after checklist is complete

  • Add input/output type hints to new functions/methods
  • Write design document if needed (Do I need to write a design document?)
  • Write/update unit tests
  • Write/update integration tests
  • Write/update functional tests if needed
  • make pr passes
  • make update-reproducible-reqs if dependencies were changed
  • Write documentation

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@qingchm qingchm marked this pull request as ready for review June 8, 2022 07:18
Copy link
Contributor

@mndeveci mndeveci left a comment

Choose a reason for hiding this comment

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

Looks good to me in general, left a small comment there

Comment on lines 80 to 84
definition_path = Path(self._build_context.base_dir).joinpath(definition_file)
if not self._build_context.use_base_dir:
child_stack = Stack.get_stack_by_full_path(ResourceIdentifier(api_identifier).stack_path, self._stacks)
if child_stack:
definition_path = Path(child_stack.location).parent.joinpath(definition_file)
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems duplicated lines with SFN flow. Can we move this one into a common function so that we can re-use it in sfn sync flow as well?
Can we also add some debug logs if we need to investigate an issue in the future?

@qingchm qingchm requested a review from mndeveci June 9, 2022 02:20
Comment on lines 321 to 322
# A helper method used by non-function sync flows to resolve definition file path
# that are relative to the child stack to absolute path for nested stacks
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: let's put this as method documentation rather than a comment here.

@mndeveci mndeveci enabled auto-merge (squash) June 9, 2022 23:49
@mndeveci mndeveci merged commit 4b4e6e4 into aws:develop Jun 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants