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

Remove deprecated Datadog::HTTPTransport #782

Merged
merged 4 commits into from
Jul 23, 2019

Conversation

delner
Copy link
Contributor

@delner delner commented Jul 17, 2019

In #628, we refactored the transport layer in which we added a new Datadog::Transport::HTTP::Client and deprecated the old Datadog::HTTPTransport.

In keeping with our plans to deprecate, this pull request removes the old Datadog::HTTPTransport entirely, for the 0.26.0 release.

@delner delner added breaking-change Involves a breaking change dev/refactor Involves refactoring existing components labels Jul 17, 2019
@delner delner added this to the 0.26.0 milestone Jul 17, 2019
@delner delner requested a review from brettlangdon July 17, 2019 21:32
@delner delner self-assigned this Jul 17, 2019
@@ -1,101 +0,0 @@
require 'spec_helper'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed this integration test since it overlaps with the HTTP adapter integration tests, which are responsible for tagging the request with the correct HTTP headers. Hence, this is considered obsolete.

@@ -1,64 +0,0 @@
require 'spec_helper'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed this integration test since it overlaps with the HTTP adapter integration tests, which are responsible for tagging the request with the correct HTTP headers. Hence, this is considered obsolete.

Copy link
Member

@brettlangdon brettlangdon left a comment

Choose a reason for hiding this comment

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

minor question, but lgtm

require 'ddtrace/transport/http'
require 'ddtrace/transport/http/adapters/net'

RSpec.describe 'Adapters::Net integration tests' do
Copy link
Member

Choose a reason for hiding this comment

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

Were these missing before or transplanted from removed spec files?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They never existed: we had similar specs in meta_header_spec.rb and http_integration_spec.rb but none that explicitly tested Net::HTTP adapter in such a way.

@delner delner merged commit 873841e into 0.26-dev Jul 23, 2019
@delner delner deleted the refactor/remove_httptransport branch July 23, 2019 14:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change Involves a breaking change dev/refactor Involves refactoring existing components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants