-
Notifications
You must be signed in to change notification settings - Fork 764
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
New Adapter: Kueez #4038
New Adapter: Kueez #4038
Conversation
Code coverage summaryNote:
kueezrtbRefer here for heat map coverage report
|
@@ -0,0 +1,20 @@ | |||
endpoint: "https://prebidsrvr.kueezrtb.com/openrtb/" |
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.
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.
@przemkaczmarek
You need to add the cId
to the request
prebid-server/adapters/kueezrtb/kueezrtb.go
Lines 54 to 62 in bf7ab42
requestData := &adapters.RequestData{ | |
Method: "POST", | |
Uri: fmt.Sprintf("%s%s", a.endpoint, url.QueryEscape(cId)), | |
Body: requestJSON, | |
Headers: headers, | |
ImpIDs: []string{imp.ID}, | |
} | |
requests = append(requests, requestData) |
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.
@przemkaczmarek
Can we approve please?
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.
userSync: | ||
iframe: | ||
url: https://sync.kueezrtb.com/api/user/html/62ce79e7dd15099534ae5e04?pbs=true&gdpr={{.GDPR}}&gdpr_consent={{.GDPRConsent}}&us_privacy={{.USPrivacy}}&redirect={{.RedirectURL}}&gpp={{.GPP}}&gpp_sid={{.GPPSID}} | ||
userMacro: ${userId} |
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.
adapters/kueezrtb/params_test.go
Outdated
} | ||
|
||
var invalidParams = []string{ | ||
`{"cId": 3898334}`, |
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.
what happend if You add:
var invalidParams = []string{
{}
,
{"cId": 123}
,
{"cId": true}
,
{"cId": null}
,
{"cId": ""}
,
{"cId": " "}
,
{"cId": "cid_with_invalid_chars!@#"}
,
{"cId": "valid_cid", "extra_field": "unexpected"}
,
{"cId": "a_very_long_connection_id_exceeding_the_maximum_allowed_length_..."}
,
[]
,
["cId"]
,
"string_value"
,
42
,
true
,
{"cId": "valid_cid"
,
{"cId":}
,
{"cid": "valid_cid"}
,
}
please add more invalidParams
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.
Added more..
Code coverage summaryNote:
kueezrtbRefer here for heat map coverage report
|
Hi @saar120, we noticed that your adapter is basically the same as the Vidazoo adapter. We would like to point out that aliasing an adapter may be an option for you and is preferable if you and another bidder share the same server. You can read more about aliasing here: aliasing an adapter. Please let us know your thoughts on whether this is a possibility for you. |
Hey @bsardo |
@przemkaczmarek are we waiting for an additional approval? we would really appreciate if it could be included in the following release |
@assa-kariv In our policy, we require two approvals. |
@przemkaczmarek yeah I am aware of that thanks for tagging another approver |
@bsardo |
@bsardo @przemkaczmarek please assist here |
@bsardo can you please update what is the approval status? |
New adapters must also open a PR against the Prebid Docs repository here: https://github.com/prebid/prebid.github.io/ Please see the User Documentation section of the new adapter developer guide. |
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.
Thank you for the contribution! Code looks good to me. I added a comment about optional optimization. Please also add a documentation PR.
Thank you!
…d (kueez.json validation file).
Code coverage summaryNote:
kueezrtbRefer here for heat map coverage report
|
@VeronikaSolovei9 @przemkaczmarek |
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.
Thank you for the changes! Approved
prebid/prebid.github.io#5814