prebid-server icon indicating copy to clipboard operation
prebid-server copied to clipboard

Adhese: Switch to openrtb2 endpoint

Open mefjush opened this issue 1 year ago • 7 comments

Switch the Adhese adapter from /json to /openrtb2 endpoint. This is to keep our go adapter in line with with prebid-server-java Adhese adapter.

mefjush avatar Aug 16 '24 13:08 mefjush

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, f0318749ad8dde7bc2191e015d1015d29f551235

adhese

Refer here for heat map coverage report

github.com/prebid/prebid-server/v2/adapters/adhese/adhese.go:25:	makeSlot		100.0%
github.com/prebid/prebid-server/v2/adapters/adhese/adhese.go:29:	MakeRequests		85.7%
github.com/prebid/prebid-server/v2/adapters/adhese/adhese.go:106:	inferBidTypeFromImp	100.0%
github.com/prebid/prebid-server/v2/adapters/adhese/adhese.go:123:	MakeBids		72.7%
github.com/prebid/prebid-server/v2/adapters/adhese/adhese.go:199:	Builder			100.0%
total:									(statements)		82.9%

github-actions[bot] avatar Aug 16 '24 13:08 github-actions[bot]

@SyntaxNode It got delayed a bit because of some internal testing with the new adapter, the techlead from Adhese has re-opened the PR.

Sorry for the late reply!

skonky avatar Aug 19 '24 09:08 skonky

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, b2c13e2782541f603eab2cbc8821c50e5543f817

adhese

Refer here for heat map coverage report

github.com/prebid/prebid-server/v2/adapters/adhese/adhese.go:24:	makeSlot		100.0%
github.com/prebid/prebid-server/v2/adapters/adhese/adhese.go:28:	MakeRequests		85.7%
github.com/prebid/prebid-server/v2/adapters/adhese/adhese.go:105:	inferBidTypeFromImp	100.0%
github.com/prebid/prebid-server/v2/adapters/adhese/adhese.go:122:	MakeBids		78.1%
github.com/prebid/prebid-server/v2/adapters/adhese/adhese.go:191:	Builder			100.0%
total:									(statements)		85.3%

github-actions[bot] avatar Sep 17 '24 12:09 github-actions[bot]

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, 0e97d54e839b54e07df9a0e43c391ad7fad401f8

adhese

Refer here for heat map coverage report

github.com/prebid/prebid-server/v2/adapters/adhese/adhese.go:24:	makeSlot		100.0%
github.com/prebid/prebid-server/v2/adapters/adhese/adhese.go:28:	MakeRequests		85.7%
github.com/prebid/prebid-server/v2/adapters/adhese/adhese.go:105:	inferBidTypeFromImp	100.0%
github.com/prebid/prebid-server/v2/adapters/adhese/adhese.go:122:	MakeBids		78.1%
github.com/prebid/prebid-server/v2/adapters/adhese/adhese.go:191:	Builder			100.0%
total:									(statements)		85.3%

github-actions[bot] avatar Sep 17 '24 12:09 github-actions[bot]

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, 8f5225f769a6a0125d7e691dfa30d75cff29fcdf

adhese

Refer here for heat map coverage report

github.com/prebid/prebid-server/v2/adapters/adhese/adhese.go:24:	makeSlot		100.0%
github.com/prebid/prebid-server/v2/adapters/adhese/adhese.go:28:	MakeRequests		85.7%
github.com/prebid/prebid-server/v2/adapters/adhese/adhese.go:105:	inferBidTypeFromImp	92.9%
github.com/prebid/prebid-server/v2/adapters/adhese/adhese.go:133:	MakeBids		78.1%
github.com/prebid/prebid-server/v2/adapters/adhese/adhese.go:202:	Builder			100.0%
total:									(statements)		85.0%

github-actions[bot] avatar Sep 17 '24 12:09 github-actions[bot]

Hi @onkarvhanumante I think we've addressed (or replied to) all the comments in the PR. Could we have another round of review please?

mefjush avatar Sep 24 '24 17:09 mefjush

Hi @mefjush, I'll see if I can get this re-reviewed. Going forward, can you please push new commits instead of force pushing? And if you need to resolve conflicts, please merge with master. This will save reviewers a lot of time as they can then just look at what changed instead of having to re-review the entire PR. All of your commits will get squashed at the end when we merge. Thanks!

bsardo avatar Sep 26 '24 13:09 bsardo

@bsardo any news here yet? We're looking to get this deployed since our current adapter is broken. Thanks!

westerschmal avatar Oct 08 '24 10:10 westerschmal

@bsardo or @onkarvhanumante when would you have time for a review here? Apologies for pushing, but its a blocker for us.

westerschmal avatar Oct 22 '24 07:10 westerschmal

Looking

bsardo avatar Oct 22 '24 16:10 bsardo

Hi @mefjush, 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!

bsardo avatar Nov 04 '24 16:11 bsardo

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, 90d0c3d2ddf5ffa208d2eb257dd25f1a326b6b2d

adhese

Refer here for heat map coverage report

github.com/prebid/prebid-server/v3/adapters/adhese/adhese.go:24:	makeSlot		100.0%
github.com/prebid/prebid-server/v3/adapters/adhese/adhese.go:28:	MakeRequests		81.2%
github.com/prebid/prebid-server/v3/adapters/adhese/adhese.go:113:	inferBidTypeFromImp	92.9%
github.com/prebid/prebid-server/v3/adapters/adhese/adhese.go:141:	MakeBids		78.1%
github.com/prebid/prebid-server/v3/adapters/adhese/adhese.go:210:	Builder			100.0%
total:									(statements)		83.3%

github-actions[bot] avatar Nov 07 '24 22:11 github-actions[bot]

@bsardo master has been merged into the branch I ensured v2 is not used anymore, I also fixed the test suite. Could you give it another look? Thanks :)

skonky avatar Nov 07 '24 22:11 skonky

@bsardo Hello! Checking in to see if there’s any feedback on this PR. Appreciate your time whenever possible! 🙏

skonky avatar Nov 14 '24 13:11 skonky

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, ce3e470e6fd3923674a79822a468c6d6518a1bef

adhese

Refer here for heat map coverage report

github.com/prebid/prebid-server/v3/adapters/adhese/adhese.go:24:	makeSlot		100.0%
github.com/prebid/prebid-server/v3/adapters/adhese/adhese.go:28:	MakeRequests		80.0%
github.com/prebid/prebid-server/v3/adapters/adhese/adhese.go:109:	inferBidTypeFromImp	92.9%
github.com/prebid/prebid-server/v3/adapters/adhese/adhese.go:137:	MakeBids		78.1%
github.com/prebid/prebid-server/v3/adapters/adhese/adhese.go:206:	Builder			100.0%
total:									(statements)		82.9%

github-actions[bot] avatar Nov 18 '24 10:11 github-actions[bot]

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, 53b2e982390af7aa426aa0f6b98bf923d7aa4f60

adhese

Refer here for heat map coverage report

github.com/prebid/prebid-server/v3/adapters/adhese/adhese.go:24:	makeSlot		100.0%
github.com/prebid/prebid-server/v3/adapters/adhese/adhese.go:28:	MakeRequests		80.0%
github.com/prebid/prebid-server/v3/adapters/adhese/adhese.go:109:	inferBidTypeFromImp	92.9%
github.com/prebid/prebid-server/v3/adapters/adhese/adhese.go:137:	MakeBids		78.1%
github.com/prebid/prebid-server/v3/adapters/adhese/adhese.go:206:	Builder			100.0%
total:									(statements)		82.9%

github-actions[bot] avatar Nov 18 '24 10:11 github-actions[bot]

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, bc41b272b028add6a733436ac9123009446f3879

adhese

Refer here for heat map coverage report

github.com/prebid/prebid-server/v3/adapters/adhese/adhese.go:24:	makeSlot		100.0%
github.com/prebid/prebid-server/v3/adapters/adhese/adhese.go:28:	MakeRequests		80.0%
github.com/prebid/prebid-server/v3/adapters/adhese/adhese.go:109:	inferBidTypeFromImp	92.9%
github.com/prebid/prebid-server/v3/adapters/adhese/adhese.go:137:	MakeBids		78.1%
github.com/prebid/prebid-server/v3/adapters/adhese/adhese.go:206:	Builder			100.0%
total:									(statements)		82.9%

github-actions[bot] avatar Nov 19 '24 09:11 github-actions[bot]

@SyntaxNode @bsardo Thanks alot for reviewing, I have implemented your suggestions and feedback. Is there anything else you want to be changed or are we OK to merge now :)?

skonky avatar Nov 19 '24 09:11 skonky

Not for this PR, but so that you know. MakeRequests() returns a slice of requests. You can loop over the requests in the Imp[] and make one request per imp. PBS core will then send all those requests to your server. For each response it receives, it will call MakeBids() to get the bid from the response. It will then package all the bids together, allowing you to support multi Imp requests with very little work.

hhhjort avatar Nov 21 '24 22:11 hhhjort

Not for this PR, but so that you know. MakeRequests() returns a slice of requests. You can loop over the requests in the Imp[] and make one request per imp. PBS core will then send all those requests to your server. For each response it receives, it will call MakeBids() to get the bid from the response. It will then package all the bids together, allowing you to support multi Imp requests with very little work.

@hhhjort Yeah I would like to make a follow up PR for these improvements, our techlead @mefjush Is currently on leave but I will discuss this and the other points with him.

skonky avatar Nov 25 '24 09:11 skonky

bump 👀

skonky avatar Dec 02 '24 09:12 skonky

What do you think @SyntaxNode @bsardo @onkarvhanumante @Sonali-More-Xandr

skonky avatar Dec 06 '24 11:12 skonky

Think we can merge this before the EOY ?

skonky avatar Dec 17 '24 09:12 skonky

Think we can merge this before the EOY ?

I think so. @VeronikaSolovei9 is reviewing it.

bsardo avatar Dec 17 '24 15:12 bsardo

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, 2442c2e4660ba87e3a48c6d425b162ebb0f40452

adhese

Refer here for heat map coverage report

github.com/prebid/prebid-server/v3/adapters/adhese/adhese.go:24:	makeSlot		100.0%
github.com/prebid/prebid-server/v3/adapters/adhese/adhese.go:28:	MakeRequests		82.1%
github.com/prebid/prebid-server/v3/adapters/adhese/adhese.go:103:	inferBidTypeFromImp	92.9%
github.com/prebid/prebid-server/v3/adapters/adhese/adhese.go:131:	MakeBids		78.1%
github.com/prebid/prebid-server/v3/adapters/adhese/adhese.go:200:	Builder			100.0%
total:									(statements)		83.8%

github-actions[bot] avatar Dec 17 '24 18:12 github-actions[bot]

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, 46122f8b09a4c8a3feedfc4f918eaf1d81138f81

adhese

Refer here for heat map coverage report

github.com/prebid/prebid-server/v3/adapters/adhese/adhese.go:24:	makeSlot		100.0%
github.com/prebid/prebid-server/v3/adapters/adhese/adhese.go:28:	MakeRequests		79.2%
github.com/prebid/prebid-server/v3/adapters/adhese/adhese.go:95:	inferBidTypeFromImp	92.9%
github.com/prebid/prebid-server/v3/adapters/adhese/adhese.go:123:	MakeBids		80.0%
github.com/prebid/prebid-server/v3/adapters/adhese/adhese.go:186:	Builder			100.0%
total:									(statements)		83.8%

github-actions[bot] avatar Jan 06 '25 09:01 github-actions[bot]

@VeronikaSolovei9 Thanks again for the review and best wishes for 2025 :)

skonky avatar Jan 06 '25 10:01 skonky

Hi @skonky! Thank you for all your patience and updates! Happy New 2025 year! There is one little thing to fix and one thing to consider and I'll approve it.

VeronikaSolovei9 avatar Jan 08 '25 04:01 VeronikaSolovei9

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, 05c081ca1ac64e8b7ae2a4584490c692fc17a08d

adhese

Refer here for heat map coverage report

github.com/prebid/prebid-server/v3/adapters/adhese/adhese.go:24:	makeSlot		100.0%
github.com/prebid/prebid-server/v3/adapters/adhese/adhese.go:28:	MakeRequests		80.0%
github.com/prebid/prebid-server/v3/adapters/adhese/adhese.go:95:	inferBidTypeFromImp	92.9%
github.com/prebid/prebid-server/v3/adapters/adhese/adhese.go:123:	MakeBids		80.0%
github.com/prebid/prebid-server/v3/adapters/adhese/adhese.go:188:	Builder			100.0%
total:									(statements)		84.0%

github-actions[bot] avatar Jan 08 '25 09:01 github-actions[bot]

@hhhjort one more time 😬

skonky avatar Jan 09 '25 09:01 skonky