Skip to content
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 video - Override parameters inside createImpressionTemplate #1295

Closed
neririchter opened this issue May 10, 2020 · 7 comments
Closed
Assignees

Comments

@neririchter
Copy link

neririchter commented May 10, 2020

It seems that when using stored imps, the function of: "createImpressionTemplate" inside Video_auction class overrides all parameters except a few params:

func createImpressionTemplate(imp openrtb.Imp, video *openrtb.Video) openrtb.Imp {
imp.Video = &openrtb.Video{}
imp.Video.W = video.W
imp.Video.H = video.H
imp.Video.Protocols = video.Protocols
imp.Video.MIMEs = video.MIMEs
return imp
}
but if I wish to use for example Imp.video.playbackmethod or any other video parameters i can not.
not from the stored imps and not from the request itself, because its been override.

@SyntaxNode
Copy link
Contributor

Can you please take a look at this?
@camrice @jmaynardxandr @VeronikaSolovei9

@VeronikaSolovei9
Copy link
Contributor

VeronikaSolovei9 commented May 11, 2020

Initial bug fix PR: #1297
Please note, we don't support stored impressions in video endpoint. The only way to pass data is request body.
According to specification we only expect those 4 params in video object. This fix extends video data to all data we have in openrtb.Video object.
@SyntaxNode @camrice @jmaynardxandr

@neririchter
Copy link
Author

neririchter commented May 12, 2020

I am a bit confused when you say we don't support stored impressions in video endpoint, because if I take a look at the server documentation I am required to create pod configId that is a stored imp, and as part of the stored imp, an empty video object is been stored.
and I understand we must validate those 4 params but we can not block other openRtb params coming from the request to pass to adapters.

@VeronikaSolovei9
Copy link
Contributor

@neririchter pod id is a config id where we store only extension part of impression(in most cases bidder related parameters), not the entire impression. Video object validation is now happening in validateVideoRequest function and it validates only MIMEs and Protocols. Video.W and Video.H are optional. I agree, passing other video related parameters make sense.

@neririchter
Copy link
Author

@VeronikaSolovei9 understood, thank you for the detailed response.

@SyntaxNode
Copy link
Contributor

@neririchter Does PR #1297 resolve all of your concerns? If so, can we please close the issue?

@neririchter
Copy link
Author

@SyntaxNode yes, thank you, this PR, resolve all my concerns

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants