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 docker engine v20.10.6 compatibility #1217
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good overall, just one minor comment about potential improvement to readability/understandability
'port', | ||
containerId, | ||
]) | ||
const containerPort = containerPortBinding.split(':')[1] | ||
for (const line of dockerPortOutput.split('\n')) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be great to add a comment with explanation as to why it's parsed this way so when someone later looks at it will right away understand what's going on, otherwise it might be hard to comprehend
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for comment.
I added comments to help someone understand it.
f9bbc01
to
e234826
Compare
Description
Add Handling for IPv6 line of
docker port
output.Starting with docker engine v20.10.6,
docker port
outputs not only IPv4 mappings but also IPv6 mappings.e.g.:
https://docs.docker.com/engine/release-notes/#20106
The container port parsing has been broken by the IPv6 mapping line.
Motivation and Context
Failing tests on master, visible e.g. here: https://github.com/dherault/serverless-offline/runs/2438945825?check_suite_focus=true
How Has This Been Tested?
Automated tests.