Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ci: add secondary artifact support to PR build #619

Merged
merged 2 commits into from
Jan 15, 2020

Conversation

jahkeup
Copy link
Member

@jahkeup jahkeup commented Dec 30, 2019

Issue #, if available:

#604

Description of changes:

Added SecondaryArtifacts section to the Pull Request build CloudFormation Stack with the referenced ArtifactIdentifier to allow for uploading "meta" artifacts.

I've updated a stack in my own account and saw the change reflected. In the interest of keeping the artifacts separate from one another I've also renamed the output location for the Primary Artifacts - I didn't check to see if leaving it empty would cause CodeBuild to upload and later unpack from a shared / where both the Secondary Artifacts lived (eg: / & /meta) and chose to avoid the situation altogether by giving them each their own key prefix (/build & /meta).

Update: In the updated stack, a kicked-off build now has the primary and the secondary artifacts linked to.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@jahkeup
Copy link
Member Author

jahkeup commented Jan 6, 2020

@etungsten this may be reasonable to tack along with the work to reorganize the Stacks (maybe?), do you think we should wait until those changes are merged / well-underway to coordinate making this change?

Copy link
Contributor

@zmrow zmrow left a comment

Choose a reason for hiding this comment

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

💴

@etungsten
Copy link
Contributor

@etungsten this may be reasonable to tack along with the work to reorganize the Stacks (maybe?), do you think we should wait until those changes are merged / well-underway to coordinate making this change?

I don't think we need to wait. These changes are small enough to incorporate.

@jahkeup
Copy link
Member Author

jahkeup commented Jan 7, 2020

@etungsten these changes will need to be made to the other stacks as well, if making that change in this PR isn't an issue then I'll go ahead and do that.

Without this change, the script was prevented from writing in the
correct build/meta directory as intended and was instead writing to
the current directory.
@jahkeup jahkeup marked this pull request as ready for review January 15, 2020 20:27
@jahkeup
Copy link
Member Author

jahkeup commented Jan 15, 2020

Rebased on merged ci-containers changes - no content changes here, will merge.

@jahkeup jahkeup merged commit abdcef8 into develop Jan 15, 2020
@jahkeup jahkeup deleted the ci-secondary-artifacts branch January 15, 2020 20:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants