-
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
Update Cox Bid Adapter For 1.0+ #2446
Conversation
Please submit a PR to update https://github.com/prebid/prebid.github.io/blob/master/dev-docs/bidders/cox.md to add prebid_1_0_supported : true into the header section. |
I'm trying to figure out if the state variables in helper object are functioning as global variables. I think they are. If the bidder gets instantiated twice, it looks to me that both instances of the bidder are going to refer to the same helper object. Is that what is going to happen or am I getting this wrong? |
@reynold-cox if you could answer my questions bout helper object variables, that would get this moving along |
Hi Mike
Sorry I didn't get back to you earlier on this. The state variables (env,
tag, and placementMap) are local to the closure that's bound to the helper
object. However, if different instances of the bidder reference the same
instance of the coxBidAdapter module (i.e. there is only ever one copy of
the helper object), then yes that state info will run into issues. If
that's the case, then we can:
1. Have state info be keyed by Auction ID. So if the helper object
participates in multiple auctions - it can keep track of the state for each.
...OR...
2. We can put the state info as another entry in the exported spec object
if a new bidder instance means a new spec object is created that is bound
to a single auction.
The purpose of the state info was to minimize code changes in our ad
server, and to keep the response bytes down. Please advise. Thanks!
Reynold
…On Thu, Apr 26, 2018 at 6:31 PM, Mike Chowla ***@***.***> wrote:
I'm trying to figure out if the state variables in helper object are
functioning as global variables. I think they are. If the bidder gets
instantiated twice, it looks to me that both instances of the bidder are
going to refer to the same helper object.
Is that what is going to happen or am I getting this wrong?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#2446 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AUWuTqeYJZHvDRUAQH8dU1hugB9wx6cpks5tsnUEgaJpZM4TgO88>
.
|
@reynold-cox to pass state between buildRequests & interpretResponse you can additional fields to return object from buildRequests as that object will get passed back to interpretResponse |
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.
3 things to fix:
(1) Global variables in helper object
(2) Access-Control-Allow-Origin error
(3) getUserSyncs() error on body parameter
modules/coxBidAdapter.js
Outdated
}; | ||
|
||
// State variables | ||
let env = ''; |
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.
These variables are acting as global variables.
Bidder Requirements:
Support multiple instances: All adapters must support the creation of multiple concurrent instances. This means, for example, that adapters cannot rely on mutable global variables.
Hi Mike
I'll do that and start a new PR tomorrow. Thanks!
Reynold
…On Mon, Apr 30, 2018 at 4:13 PM, Mike Chowla ***@***.***> wrote:
@reynold-cox <https://github.com/reynold-cox> to pass state between
buildRequests & interpretResponse you can additional fields to return
object from buildRequests as that object will get passed back to
interpretResponse
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#2446 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AUWuTvcgIh1B_msTsF18dyguGpWH5vA1ks5tt5qBgaJpZM4TgO88>
.
|
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. |
Sorry about the delay. Thanks. |
* Bid adapter for 1.0+ * Tests for cox bid adapter * Corrected how state is handled * Added test for multiple bids
* Bid adapter for 1.0+ * Tests for cox bid adapter * Corrected how state is handled * Added test for multiple bids
Type of change
Description of change
Updating the Cox bid adapter to be 1.0+ compliant.