TTD Bid Adapter: Remove the test HTTP2 endpoint and set alwaysHasCapacity#14665
TTD Bid Adapter: Remove the test HTTP2 endpoint and set alwaysHasCapacity#14665patmmccann merged 3 commits intoprebid:masterfrom
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d989c8a571
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| return params.customBidderEndpoint | ||
| } | ||
|
|
There was a problem hiding this comment.
Preserve
useHttp2 endpoint selection behavior
Removing the params.useHttp2 branch from selectEndpoint makes existing useHttp2: true configurations silently route to https://direct.adsrvr.org/... instead of https://d2.adsrvr.org/.... This is a backward-incompatible behavior change in the adapter contract (also reflected by the existing test/spec/modules/ttdBidAdapter_spec.js case for the HTTP2 endpoint), so callers that rely on the flag now get different production routing with no warning.
Useful? React with 👍 / 👎.
Pull Request Test Coverage Report for Build 23869817550Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
|
I'd like TTD engineering to review this change before it gets merged. @jlquaccia please do not approve until we've had a chance to review. |
|
@robin-crazygames can you set alwaysHasCapacity |
|
@patmmccann I don't understand what you mean with
You mean I have to do something for this PR (although I think you already did it yourself), or do you mean something else? |
|
yeah i did it, ty :) |
decaffeinatedio
left a comment
There was a problem hiding this comment.
lgtm -- thank you for opening this!
|
thanks for taking a look @decaffeinatedio ! merging now |
Type of change
Bugfix
Feature
New bidder adapter
Updated bidder adapter
Code style update (formatting, local variables)
Refactoring (no functional changes, no api changes)
Build related changes
CI related changes
Does this change affect user-facing APIs or examples documented on http://prebid.org?
Other
Description of change
Note that I have no affiliation with TTD.
Based on prebid/prebid.github.io#6499 (comment), and based on the fact that experiments with the endpoint I removed in this commit resulted in 25% less revenue, I propose to remove that secondary end point from the code base.
Other information