New Adapter: Brave #2396

thebraveio · 2022-09-28T18:38:32Z

No description provided.

VeronikaSolovei9

LGTM!

adapters/brave/brave.go

VeronikaSolovei9 · 2022-10-05T17:56:41Z

adapters/brave/bravetest/supplemental/invalid-response.json

@@ -0,0 +1,98 @@
+


Super nitpick: please remove empty lines from json files where they are present.

VeronikaSolovei9

I added some minor comments.
Other than that code looks good, local testing looks good as well.
I'll re-approve it again after you make changes :)

bretg · 2022-10-06T21:20:44Z

docs PR prebid/prebid.github.io#4039

Please address comments

AlexBVolcy · 2022-10-12T19:29:46Z

There is a PR #2370 that is being merged today that will cause conflicts to new adapter PRs.

To resolve these conflicts, you'll need to do the following:

In brave.go, update the Builder() function signature to include a new parameter server config.Server

In brave_test.go, update any test that includes a call to Builder() to pass a config.Server object, to that call (i.e. config.Server{ExternalUrl: "http://hosturl.com", GvlID: 1, DataCenter: "datacenter"})

After the merge, you can look at other adapters to see how the server object is implemented as an example.

SyntaxNode

Nice test coverage. Left a few comments about multi impression support and bid type detection.

adapters/brave/brave.go

SyntaxNode · 2022-10-12T21:21:02Z

adapters/brave/brave.go

+			} else if imp.Native != nil {
+				mediaType = openrtb_ext.BidTypeNative
+			}
+			return mediaType


Although many adapters do this, we consider it an anti-pattern. Does your bidding server return the media type in the response? Such as the new OpenRTB 2.6 mtype field?

no, we using 2.5 and response with adm without mtype, that's why we have to recognise type by using req parameter

SyntaxNode · 2022-10-12T21:21:36Z

adapters/brave/brave_test.go

+
+func TestJsonSamples(t *testing.T) {
+	bidder, buildErr := Builder(openrtb_ext.BidderBrave, config.Adapter{
+		Endpoint: "http://point.braveglobal.tv/?t=3&partner={{.PublisherID}}"})


Please consider using an obviously fake url for testing. There is no need for this to match the real endpoint and might be harder to maintain if you need to change it in the future.

oh ok, got it , thanks

exchange/adapter_builders.go

SyntaxNode · 2022-10-12T21:26:13Z

This code is identical to #2399. If these are aliases of the same underlying bidder, it's better to implement it just once and use an alias. Less code to maintain.

bsardo · 2022-10-26T17:14:03Z

This code is identical to #2399. If these are aliases of the same underlying bidder, it's better to implement it just once and use an alias. Less code to maintain.

Hi @thebraveio, just checking in to see if you can shed some light on this. Thanks!

thebraveio · 2022-11-04T15:50:14Z

Added commit with fixes

bsardo · 2022-11-07T16:19:03Z

Hi @thebraveio, can you merge with master when you get a chance? Your code is failing the go 1.19 validation checks due to go fmt changes that were introduced with the recent upgrade to go 1.19 on master.

Also your code is failing the validate-merge step due to some recent changes we made on master regarding the ORTB library. You'll need to update the reference to the mxmcherry version of the library to the prebid version.

thebraveio · 2022-11-18T12:43:53Z

updated, check it again pls

bsardo · 2022-11-18T16:19:07Z

@thebraveio you still have errors to fix. The tests aren't passing because you have references to video heroes data structures in this PR.

thebraveio · 2022-11-18T19:18:17Z

@thebraveio you still have errors to fix. The tests aren't passing because you have references to video heroes data structures in this PR.

yes, my mistake, sorry

thebraveio · 2022-11-28T13:18:48Z

hi Guys, could you review the pull ? it just on pending for review

VeronikaSolovei9 · 2022-11-29T22:12:20Z

go.sum

@@ -29,12 +29,18 @@ cloud.google.com/go v0.94.1/go.mod h1:qAlAugsXlC+JWO+Bke5vCtc9ONxjQT3drlTTnAplMW
 cloud.google.com/go v0.97.0/go.mod h1:GF7l59pYBVlXQIBLx3a761cZ41F9bBH3JUlihCt2Udc=
 cloud.google.com/go v0.98.0/go.mod h1:ua6Ush4NALrHk5QXDWnjvZHN93OuF0HfuEPq9I1X0cM=
 cloud.google.com/go v0.99.0/go.mod h1:w0Xx2nLzqWJPuozYQX+hFfCSI8WioryfRDzkoI/Y2ZA=
+cloud.google.com/go v0.100.2/go.mod h1:4Xra9TjzAeYHrl5+oeLlzbM2k3mjVhZh4UqTZ//w99A=


Please do not push changes in go.sum if you didn't modify go.mod. This should not be a part of this PR, there are no new libraries added

UPD: discussed it with the team, it's ok to have changes in go.sum, no actions needed

AlexBVolcy

LGTM

bsardo · 2023-01-04T18:13:29Z

Hi @thebraveio, would you mind rolling back the changes you made to go.sum?

bsardo · 2023-01-17T18:18:30Z

Hi @thebraveio, just checking in to see if you saw my earlier request to roll back changes to go.sum. No rush 🙂

… 1.19 (prebid#2428)

videoheroes · 2023-01-20T11:10:34Z

sorry for a delay, I checkout file to version was pulled before push

bsardo · 2023-01-20T15:44:14Z

@videoheroes, @thebraveio, no worries. It looks like you have two more lines in go.sum that involve the github.com/go-sql-driver/mysql package to restore and that is why the tests are failing.

videoheroes · 2023-01-24T16:00:22Z

yes, seems like I need to update go.* to #2356

…id#2356)

thebraveio · 2023-01-25T10:05:11Z

Hi Guys ! I see we do not pass security auto test, do you know where it comes from, as we do not touch any core code it can be versions problem again ?

AlexBVolcy

LGTM! Discussed with the team today, you don't have to worry about the Trivy security failure, as we can still approve and merge in a PR if that's the only thing wrong with it.

VeronikaSolovei9

LGTM

ÑNew Adapter: Brave

76c972d

bsardo requested review from bsardo and VeronikaSolovei9 September 29, 2022 17:23

bsardo assigned bsardo and VeronikaSolovei9 Sep 29, 2022

SyntaxNode added the port to java label Oct 1, 2022

VeronikaSolovei9 previously approved these changes Oct 5, 2022

View reviewed changes

VeronikaSolovei9 reviewed Oct 5, 2022

View reviewed changes

adapters/brave/brave.go Outdated Show resolved Hide resolved

VeronikaSolovei9 reviewed Oct 5, 2022

View reviewed changes

SyntaxNode reviewed Oct 12, 2022

View reviewed changes

SyntaxNode mentioned this pull request Oct 12, 2022

New Adapter: VideoHeroes #2399

Merged

added fixes and server config support

3ad29a2

bsardo removed their assignment Nov 7, 2022

root added 2 commits November 18, 2022 12:34

Merge branch 'master' into brave_serveradapter

66314fb

merged with master

908eb23

fixind mistake with ext

a477ef4

bsardo requested a review from AlexBVolcy November 21, 2022 18:57

bsardo assigned AlexBVolcy Nov 21, 2022

VeronikaSolovei9 reviewed Nov 29, 2022

View reviewed changes

VeronikaSolovei9 previously approved these changes Nov 30, 2022

View reviewed changes

AlexBVolcy previously approved these changes Dec 5, 2022

View reviewed changes

checkout go.sum file to previous version d162aa9 Update Go version to…

c3c9d13

… 1.19 (prebid#2428)

thebraveio dismissed stale reviews from AlexBVolcy and VeronikaSolovei9 via c3c9d13 January 20, 2023 11:09

checkouted go.sum file to previous version Support MySql driver (preb…

9525bd3

…id#2356)

AlexBVolcy approved these changes Jan 30, 2023

View reviewed changes

VeronikaSolovei9 approved these changes Jan 31, 2023

View reviewed changes

bsardo merged commit 30c86d4 into prebid:master Jan 31, 2023

bretg mentioned this pull request Feb 14, 2023

Port brave adapter prebid/prebid-server-java#2158

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New Adapter: Brave #2396

New Adapter: Brave #2396

thebraveio commented Sep 28, 2022

VeronikaSolovei9 left a comment

VeronikaSolovei9 Oct 5, 2022

VeronikaSolovei9 left a comment

bretg commented Oct 6, 2022

AlexBVolcy commented Oct 12, 2022

SyntaxNode left a comment

SyntaxNode Oct 12, 2022

thebraveio Nov 4, 2022

SyntaxNode Oct 12, 2022

thebraveio Nov 4, 2022

SyntaxNode commented Oct 12, 2022

bsardo commented Oct 26, 2022

thebraveio commented Nov 4, 2022

bsardo commented Nov 7, 2022

thebraveio commented Nov 18, 2022

bsardo commented Nov 18, 2022

thebraveio commented Nov 18, 2022

thebraveio commented Nov 28, 2022

VeronikaSolovei9 Nov 29, 2022

VeronikaSolovei9 Nov 30, 2022 •

edited

Loading

AlexBVolcy left a comment

bsardo commented Jan 4, 2023

bsardo commented Jan 17, 2023

videoheroes commented Jan 20, 2023

bsardo commented Jan 20, 2023

videoheroes commented Jan 24, 2023 •

edited

Loading

thebraveio commented Jan 25, 2023

AlexBVolcy left a comment

VeronikaSolovei9 left a comment

New Adapter: Brave #2396

New Adapter: Brave #2396

Conversation

thebraveio commented Sep 28, 2022

VeronikaSolovei9 left a comment

Choose a reason for hiding this comment

VeronikaSolovei9 Oct 5, 2022

Choose a reason for hiding this comment

VeronikaSolovei9 left a comment

Choose a reason for hiding this comment

bretg commented Oct 6, 2022

AlexBVolcy commented Oct 12, 2022

SyntaxNode left a comment

Choose a reason for hiding this comment

SyntaxNode Oct 12, 2022

Choose a reason for hiding this comment

thebraveio Nov 4, 2022

Choose a reason for hiding this comment

SyntaxNode Oct 12, 2022

Choose a reason for hiding this comment

thebraveio Nov 4, 2022

Choose a reason for hiding this comment

SyntaxNode commented Oct 12, 2022

bsardo commented Oct 26, 2022

thebraveio commented Nov 4, 2022

bsardo commented Nov 7, 2022

thebraveio commented Nov 18, 2022

bsardo commented Nov 18, 2022

thebraveio commented Nov 18, 2022

thebraveio commented Nov 28, 2022

VeronikaSolovei9 Nov 29, 2022

Choose a reason for hiding this comment

VeronikaSolovei9 Nov 30, 2022 • edited Loading

Choose a reason for hiding this comment

AlexBVolcy left a comment

Choose a reason for hiding this comment

bsardo commented Jan 4, 2023

bsardo commented Jan 17, 2023

videoheroes commented Jan 20, 2023

bsardo commented Jan 20, 2023

videoheroes commented Jan 24, 2023 • edited Loading

thebraveio commented Jan 25, 2023

AlexBVolcy left a comment

Choose a reason for hiding this comment

VeronikaSolovei9 left a comment

Choose a reason for hiding this comment

VeronikaSolovei9 Nov 30, 2022 •

edited

Loading

videoheroes commented Jan 24, 2023 •

edited

Loading