[Resolve #1114,#886,#491] Project Dependencies part 2: Resolvable role_arn and template_bucket_name #1153
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
jfalkenstein
changed the title
jfalkenstein
changed the title
jfalkenstein
changed the title
jfalkenstein
changed the title
…es-p1-property-refactor
…-dependencies-p2-resolvable-value-property
…-dependencies-p2-resolvable-value-property
jfalkenstein
changed the base branch from
feature/project-dependencies
to
jf/proj-dependencies-p1-property-refactor
…-dependencies-p2-resolvable-value-property
jfalkenstein
changed the title
|
Alright @zaro0508 , @zxiiro this PR is now fully ready for review. You both had made some comments on this PR previously (and I've responded to those). I've added in proper docs and integration tests on this PR, so it should now be ready for review. This PR can be merged and deployed to master without requiring any changes from parts 3 and onward. However, this PR is pointed at the Part 1 branch because it builds on the changes in that one. |
zaro0508
reviewed
Dec 20, 2021
Comment on lines
71
to
74
| If you resolve ``template_bucket_name`` using the ``!stack_output`` | ||
| resolver on a StackGroup, the stack that outputs that bucket name *cannot* be | ||
| defined in that StackGroup. Otherwise, a circular dependency will exist and Sceptre | ||
| will raise an error when attempting any Stack action. |
There was a problem hiding this comment.
Nice description of how it should not be done. Is it possible to provide an example of a property way to do it?
There was a problem hiding this comment.
Yeah, I can add another line indicating the correct way to do it.
Comment on lines
193
to
196
| You might have already considered that this might cause a circular dependency for those | ||
| dependency stacks, the ones that output the template bucket name, role arn, iam_role, or topic arns. | ||
| In order to avoid the circular dependency issue, it is important that you define these items in a | ||
| Stack that is *outside* the StackGroup you reference them in. |
There was a problem hiding this comment.
same as above. an example directory structure and files would really help.
|
@zaro0508 , I gave two examples of project structure supporting referencing project dependencies with |
zaro0508
approved these changes
Dec 24, 2021
Base automatically changed from
jf/proj-dependencies-p1-property-refactor
to
master
December 27, 2021 15:00
…es-p2-resolvable-value-property
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.
This is the second in a series of pull requests that addresses #1114 , allowing Sceptre to manage its own dependencies.
In this PR:
PR Checklist
[Resolve #issue-number].make test) are passing.pre-commit run --all-files).and description in grammatically correct, complete sentences.
Approver/Reviewer Checklist
Other Information
Guide to writing a good commit