-
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
Smart Ad Server: Fix bug when multi bids #2170
Conversation
I tried testing this with the parameters from #1908 but I'm not getting any bids back Here's the request body:
|
Hey @mike-chowla, Indeed the parameters in this PR are out dated, and were actually changed in the md file. the right ones are
They just weren't updated in the PR message at the time. Sorry about that. |
I tried with the new params and am still not getting any bids back. Response is 200 status code, content length 0 Request body:
|
Hey @mike-chowla, I don't get it, I tried with your exact request body (copy/paste) with a rest client and i get the response. I also tried with the test page locally and it works just fine. |
Hello @mike-chowla, As I said it works when i try, could you try again ? Also, If it still doesn't work could you show the Url you are calling ? |
Hello @mike-chowla I know it's a bit annoying but i don't understand why you don't have any responses. |
Hi @mike-chowla,
Thank you. |
Could you please provide a HAR file so we could compare with our test? Do you have any errors in the console? In any case for us it is a valid JSON as a response, the test doesn't return any creative to show. Thank you. |
The test parameters need to return a valid ad. HAR File: There's actually 2 issues. The 0.05 cpm you are sending back is getting put in the zero price bucket which causes ads not to display with hello_world.html. I can hack around that but you should really send a test ad back with a higher cpm to stay out of that bucket. The adapter code returns ad field as the creative and as that field is coming back as JSON, it's not a valid creative Here's what I'm getting back.
|
Hi @mike-chowla ,
Could you please test again? Thank you. |
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.
LTGM. Test ad works . 2 slots works. Would be a good idea to add a test for the multiple ad slot case
Type of change
Description of change
Support multi bids
Other information
See issue #2166