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

fix: pass the missing extra_hosts argument #6889

Merged
merged 4 commits into from
Apr 8, 2024

Conversation

gkuga
Copy link
Contributor

@gkuga gkuga commented Mar 31, 2024

Which issue(s) does this change fix?

This fixes #6888 and related to #6078.
I have confirmed that this change makes --add-host work.

Why is this change necessary?

I change to pass the extra_hosts that were not passed as arguments.
Because sam local start-api --add-host does not work.

How does it address the issue?

Pass the missing argument.

What side effects does this change have?

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.

@gkuga gkuga requested a review from a team as a code owner March 31, 2024 03:21
@github-actions github-actions bot added area/local/start-api sam local start-api command area/local/invoke sam local invoke command area/local/start-invoke pr/external stage/needs-triage Automatically applied to new issues and PRs, indicating they haven't been looked at. labels Mar 31, 2024
@lucashuy
Copy link
Contributor

lucashuy commented Apr 3, 2024

Thanks for making creating this fix! The changes look good to me, can you run a quick make format to fix any formatting issues in the source code? Looks like that is what is causing the PR jobs to fail at the moment.

@lucashuy lucashuy removed the stage/needs-triage Automatically applied to new issues and PRs, indicating they haven't been looked at. label Apr 3, 2024
@gkuga
Copy link
Contributor Author

gkuga commented Apr 4, 2024

@lucashuy Thanks for your feedback. I've run make format. Will the workflow pass?

@lucashuy
Copy link
Contributor

lucashuy commented Apr 5, 2024

Thanks for updating the PR, looks like the workflow passes! Just need one more reviewer from our team before we can merge this in.

Comment on lines +70 to +71
container_host_interface string
Optional. Interface that Docker host binds ports to
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for updating missing documentation!

@lucashuy lucashuy added this pull request to the merge queue Apr 8, 2024
Merged via the queue into aws:develop with commit 3ab9402 Apr 8, 2024
55 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug: sam local start-api --add-host does not work
3 participants