-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Prebid Core: Restore use of server-side adapter without client-side adapter #7711
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.
@jorgeluisrocha Looking at the test coverage, not a single test goes through your changes:
Can you add at least one test for the changes you did?
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
@jorgeluisrocha following up on this pr. Are you still working on this pr to add a little test coverage? |
@karimMourra can you weigh in here? this seems stalled |
Hi @patmmccann , Jorge will add unit tests to this PR when he has enough bandwidth |
@karimMourra @jorgeluisrocha following up on this pr. Is it okay if I close this pr for now and it is just reopened when bandwidth provides |
This PR is related to issue #6361 For the record, client-side bid adapters defines 4 things that could be important to server behavior:
#1-3 are ok -- either they can be defined in s2sConfig as needed or they're also defined server-side. A few bidAdapters do however, require transformBidParams() Those ones may not work well if the client-side adapter is not present. |
This was incidentally fixed in #8357; just verified that no client side adapters are needed now for interfacing with Server. |
Type of change
Description of change
Fixes a bug that requires client-side adapter codes to be imported for server-side bidding. This obviates the need to import client-side adapter specifications. Additionally, this resolves the regression introduced by the original version of this PR found in #7662.
Other information
Issue: #6361
Issue: #6388