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

New Adapter: Kobler #3904

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

New Adapter: Kobler #3904

wants to merge 6 commits into from

Conversation

TommyHPettersen
Copy link

* New Adapter: Kobler
var bidExt openrtb_ext.ExtBid
err := json.Unmarshal(bid.Ext, &bidExt)
if err == nil && bidExt.Prebid != nil {
mediaType, err := openrtb_ext.ParseBidType(string(bidExt.Prebid.Type))
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider this as a suggestion. Prebid server expects the media type to be explicitly set in the adapter response. Therefore, recommends implementing a pattern where the adapter server sets the MType field in the response to accurately determine the media type for the impression.

Copy link

Code coverage summary

Note:

  • Prebid team doesn't anticipate tests covering code paths that might result in marshal and unmarshal errors
  • Coverage summary encompasses all commits leading up to the latest one, c65ede1

kobler

Refer here for heat map coverage report

github.com/prebid/prebid-server/v2/adapters/kobler/kobler.go:26:	Builder			100.0%
github.com/prebid/prebid-server/v2/adapters/kobler/kobler.go:35:	MakeRequests		85.7%
github.com/prebid/prebid-server/v2/adapters/kobler/kobler.go:67:	MakeBids		100.0%
github.com/prebid/prebid-server/v2/adapters/kobler/kobler.go:98:	getEndpoint		100.0%
github.com/prebid/prebid-server/v2/adapters/kobler/kobler.go:106:	getMediaTypeForBid	100.0%
github.com/prebid/prebid-server/v2/adapters/kobler/kobler.go:121:	convertImpCurrency	100.0%
github.com/prebid/prebid-server/v2/adapters/kobler/kobler.go:135:	contains		50.0%
total:									(statements)		92.2%

@bsardo bsardo self-assigned this Sep 20, 2024
@TommyHPettersen
Copy link
Author

Hello @bsardo @przemkaczmarek, what can I expect as an estimate for merging this, and is there anything I can contribute with to move this forward?

@przemkaczmarek przemkaczmarek self-requested a review October 7, 2024 12:47
endpoint: "https://bid.essrtb.com/bid/prebid_server_rtb_call"
endpointCompression: gzip
maintainer:
email: [email protected]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I sent email, please confirm

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mail confirmed:
image

@@ -0,0 +1,12 @@
endpoint: "https://bid.essrtb.com/bid/prebid_server_rtb_call"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

endpoint is reachable

return a.endpoint
}

func getMediaTypeForBid(bid openrtb2.Bid) openrtb_ext.BidType {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prebid server expects the media type to be explicitly set in the adapter response. Therefore, we strongly recommend implementing a pattern where the adapter server sets the MType field in the response to accurately determine the media type for the impression.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

our adapter has a way of reading the mtype/markup type from our bidder in the current implementation.
Since we currently only support banner ads, we by default return the banner bid type if nothing is specified.

Copy link

Code coverage summary

Note:

  • Prebid team doesn't anticipate tests covering code paths that might result in marshal and unmarshal errors
  • Coverage summary encompasses all commits leading up to the latest one, c9a87a6

kobler

Refer here for heat map coverage report

github.com/prebid/prebid-server/v2/adapters/kobler/kobler.go:26:	Builder			100.0%
github.com/prebid/prebid-server/v2/adapters/kobler/kobler.go:35:	MakeRequests		87.5%
github.com/prebid/prebid-server/v2/adapters/kobler/kobler.go:70:	MakeBids		100.0%
github.com/prebid/prebid-server/v2/adapters/kobler/kobler.go:101:	getEndpoint		100.0%
github.com/prebid/prebid-server/v2/adapters/kobler/kobler.go:109:	getMediaTypeForBid	100.0%
github.com/prebid/prebid-server/v2/adapters/kobler/kobler.go:124:	convertImpCurrency	100.0%
github.com/prebid/prebid-server/v2/adapters/kobler/kobler.go:138:	contains		50.0%
total:									(statements)		92.5%

@TommyHPettersen
Copy link
Author

@przemkaczmarek Hello, is there anything blocking this for a second reviewer and is there anything I can do to help speed this process up?

przemkaczmarek
przemkaczmarek previously approved these changes Oct 21, 2024
@TommyHPettersen
Copy link
Author

Thank you for the review so far. @bsardo When do you think you have time to review this?

@bsardo
Copy link
Collaborator

bsardo commented Nov 4, 2024

Hi @TommyHPettersen, I'll give this a look as soon as the following is addressed.

We recently released PBS 3.0, more specifically v3.1.0, which updates Prebid Server package import references throughout the project from v2 to v3.
For example:

import (
    "github.com/prebid/prebid-server/v3/adapters"
)

As a result, please merge with master (no rebase) and then ensure all Prebid Server package import references in the files you’ve changed are v3 such that the test suite passes so we can resume reviewing. Thanks!

Copy link

github-actions bot commented Nov 5, 2024

Code coverage summary

Note:

  • Prebid team doesn't anticipate tests covering code paths that might result in marshal and unmarshal errors
  • Coverage summary encompasses all commits leading up to the latest one, a18ec12

kobler

Refer here for heat map coverage report

github.com/prebid/prebid-server/v3/adapters/kobler/kobler.go:26:	Builder			100.0%
github.com/prebid/prebid-server/v3/adapters/kobler/kobler.go:35:	MakeRequests		87.5%
github.com/prebid/prebid-server/v3/adapters/kobler/kobler.go:70:	MakeBids		100.0%
github.com/prebid/prebid-server/v3/adapters/kobler/kobler.go:101:	getEndpoint		100.0%
github.com/prebid/prebid-server/v3/adapters/kobler/kobler.go:109:	getMediaTypeForBid	100.0%
github.com/prebid/prebid-server/v3/adapters/kobler/kobler.go:124:	convertImpCurrency	100.0%
github.com/prebid/prebid-server/v3/adapters/kobler/kobler.go:138:	contains		50.0%
total:									(statements)		92.5%

@TommyHPettersen
Copy link
Author

Thank you for the response @bsardo, i have now updated all the package imports to v3 and the test suite passes.

Hope this helps!

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

Successfully merging this pull request may close these issues.

4 participants