-
Notifications
You must be signed in to change notification settings - Fork 618
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
[DHPA] Rebase the feature branch against the dev branch #3590
[DHPA] Rebase the feature branch against the dev branch #3590
Conversation
Removes deprecated set-output GitHub action command. Signed-off-by: Austin Vazquez <macedonv@amazon.com>
f6310d9
to
69495e9
Compare
4876121
to
0e2a387
Compare
agent/utils/ephemeral_ports.go
Outdated
// net.Listen announces on the local tcp network | ||
ln, err := net.Listen(protocol, ":"+portStr) | ||
if err != nil { | ||
log.Debugf(portUnavailableMsg, portStr, protocol) |
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.
Line 252 to add a debug log entry is the main change make in this rebase PR.
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 thought about this before and I don't think its a good idea to add this log statement. Imagine a worst case scenario where the agent checks tens of thousands of ports and does not find any port available. The agent logs (if debug log enabled) will end up having log statements per port. And this would be per container.
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.
That's a good point! I plan to keep it for now for the bug bash event, and have a follow up PR to remove it after the bug bash.
agent/utils/ephemeral_ports.go
Outdated
// net.ListenPacket announces on the local udp network | ||
ln, err := net.ListenPacket(protocol, ":"+portStr) | ||
if err != nil { | ||
log.Debugf(portUnavailableMsg, portStr, protocol) |
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.
Line 265 to add a debug log entry is the main change make in this rebase PR.
…and go version in go.mod
Moves actions/checkout from v2 to v3 to resolve NodeJS 12 deprecation warnings in CI workflows. Signed-off-by: Austin Vazquez <macedonv@amazon.com>
9d763c1
to
8c66e5b
Compare
Summary
This PR is mainly to rebase the feature branch against the dev branch.
Commits/PRs will be merged into the feature branch include
Implementation details
Merge the dev branch into the feature branch, and resolve conflicts.
Testing
New tests cover the changes: no
Description for the changelog
N/A
Licensing
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.