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

feat: add support for docker build args to the task run command #5377

Merged
merged 4 commits into from
Oct 12, 2023

Conversation

briercliffe
Copy link
Contributor

Adds the ability to pass build args into the docker build when running the copilot task run command.

Addresses #5376

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the Apache 2.0 License.

@briercliffe briercliffe requested a review from a team as a code owner October 11, 2023 04:34
@briercliffe briercliffe requested review from CaptainCarpensir and removed request for a team October 11, 2023 04:34
Copy link
Contributor

@iamhopaul123 iamhopaul123 left a comment

Choose a reason for hiding this comment

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

This is so much needed and the implementation is awesome! Thank you very much ❤️

@@ -1193,6 +1199,7 @@ func BuildTaskRunCmd() *cobra.Command {
cmd.Flags().StringVarP(&vars.groupName, taskGroupNameFlag, nameFlagShort, "", taskGroupFlagDescription)

cmd.Flags().StringVar(&vars.dockerfilePath, dockerFileFlag, defaultDockerfilePath, dockerFileFlagDescription)
cmd.Flags().StringToStringVar(&vars.dockerfileBuildArgs, dockerFileBuildArgsFlag, nil, dockerFileBuildArgsFlagDescription)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add an example to the [task run help menu] about how we can run task from dockerfile 🙏

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added an example to the help menu showing how to use the --build-args flag. Thanks for the suggestion!

@github-actions
Copy link

github-actions bot commented Oct 11, 2023

🍕 Here are the new binary sizes!

Name New size (kiB) size (kiB) Delta (%)
macOS (amd) 52052 51976 +0.15
macOS (arm) 52916 52812 +0.20
linux (amd) 45792 45716 +0.17
linux (arm) 45124 45060 +0.14
windows (amd) 43244 43168 +0.18

Copy link
Contributor

@Lou1415926 Lou1415926 left a comment

Choose a reason for hiding this comment

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

This looks great - thank you! I'm adding the DNM label for you to address iamhopaul123@'s comment!

@@ -192,6 +193,8 @@ var (
imageFlagDescription = fmt.Sprintf(`The location of an existing Docker image.
Cannot be specified with --%s or --%s.`, dockerFileFlag, dockerFileContextFlag)
dockerFileFlagDescription = fmt.Sprintf(`Path to the Dockerfile.
Cannot be specified with --%s.`, imageFlag)
dockerFileBuildArgsFlagDescription = fmt.Sprintf(`Key-value pairs converted to --build-args.
Cannot be specified with --%s.`, imageFlag)
Copy link
Contributor

Choose a reason for hiding this comment

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

Cannot be specified with --%s.`, imageFlag

aw yay thank you for adding this in the help menu message !

@Lou1415926 Lou1415926 added the do-not-merge Pull requests that mergify shouldn't merge until the requester allows it. label Oct 11, 2023
Copy link
Contributor

@Lou1415926 Lou1415926 left a comment

Choose a reason for hiding this comment

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

Thank you!!

@Lou1415926 Lou1415926 removed the do-not-merge Pull requests that mergify shouldn't merge until the requester allows it. label Oct 12, 2023
@codecov-commenter
Copy link

codecov-commenter commented Oct 12, 2023

Codecov Report

Attention: 5 lines in your changes are missing coverage. Please review.

Comparison is base (6c673bd) 69.86% compared to head (b8eea51) 69.87%.

Additional details and impacted files
@@            Coverage Diff            @@
##           mainline    #5377   +/-   ##
=========================================
  Coverage     69.86%   69.87%           
=========================================
  Files           297      297           
  Lines         45190    45198    +8     
  Branches        287      287           
=========================================
+ Hits          31574    31580    +6     
- Misses        12079    12080    +1     
- Partials       1537     1538    +1     
Files Coverage Δ
internal/pkg/cli/flag.go 90.47% <ø> (ø)
internal/pkg/cli/task_run.go 53.88% <44.44%> (-0.04%) ⬇️

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mergify mergify bot merged commit 7e7d337 into aws:mainline Oct 12, 2023
KollaAdithya pushed a commit to KollaAdithya/copilot-cli that referenced this pull request Oct 18, 2023
…5377)

Adds the ability to pass build args into the docker build when running the `copilot task run` command.


Addresses aws#5376 

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the Apache 2.0 License.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants