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

Move httpclient to ecs-agent/ and minor refactor #3966

Merged
merged 1 commit into from
Oct 16, 2023

Conversation

danehlim
Copy link
Contributor

@danehlim danehlim commented Oct 13, 2023

Summary

Move httpclient package from agent module to ecs-agent module and also perform minor refactor of the same. This is required for future changes in ecs-agent module that depend on this package. If the httpclient package is not moved, then in the future a dependency cycle could be introduced in which the ecs-agent module depends on agent module while the agent module already depends on ecs-agent module.

Implementation details

  • Moves files from agent/httpclient/ to ecs-agent/httpclient/
  • Update dependent imports accordingly
  • Minor refactor to ecs-agent/httpclient/httpclient.go
    • ecsRoundTripper struct now has 2 new struct fields agentVersion and osTypeand related New function now takes in Agent version and OS type as arguments. This is so that we avoid making assumptions about the underlying Agent (i.e., this way, we do not need to take a dependency on the agent/version and agent/config packages)
    • Change the TLS handshake timeout used to be a constant defaultTLSHandshakeTimeout instead of a hardcoded value
    • Add code comment to provide additional context/reference on where defaultDialTimeout, defaultDialKeepalive defaultTLSHandshakeTimeout constant values come from
    • Use ecsRoundTripper transport's DialContext field in favor of its Dial field, since http.Transport.Dial is deprecated

Testing

Unit, integration, and functional tests.

New tests cover the changes: no, but existing tests updated

Description for the changelog

Move httpclient to ecs-agent/ and minor refactor

Does this PR include breaking model changes? If so, Have you added transformation functions?
No

Licensing

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@danehlim danehlim marked this pull request as ready for review October 14, 2023 00:43
@danehlim danehlim requested a review from a team as a code owner October 14, 2023 00:43
yinyic
yinyic previously approved these changes Oct 16, 2023
agent/acs/updater/updater.go Outdated Show resolved Hide resolved
hozkaya2000
hozkaya2000 previously approved these changes Oct 16, 2023
@danehlim danehlim merged commit 041ca3f into aws:dev Oct 16, 2023
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.

4 participants