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.

Sign up for GitHub

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

Jump to bottom

OwnAdx: Bidder param and URL updates #3813

Merged
merged 8 commits into from
Aug 12, 2024
Merged

OwnAdx: Bidder param and URL updates #3813

merged 8 commits into from
Aug 12, 2024

Conversation

ownAdx-prebid
Copy link
Contributor

As per describe in issue #3774.
Add the appropriate changes. Please verify.

@SerhiiNahornyi SerhiiNahornyi mentioned this pull request Jul 17, 2024
Merged
SyntaxNode
SyntaxNode reviewed Jul 23, 2024
View reviewed changes
static/bidder-info/ownadx.yaml Outdated Show resolved Hide resolved
macros/macros.go Show resolved Hide resolved
@github-actions GitHub Actions
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, afee815

ownadx

Refer here for heat map coverage report

github.com/prebid/prebid-server/v2/adapters/ownadx/ownadx.go:24:	getRequestData		83.3%
github.com/prebid/prebid-server/v2/adapters/ownadx/ownadx.go:52:	createBidRequest	100.0%
github.com/prebid/prebid-server/v2/adapters/ownadx/ownadx.go:57:	buildEndpointURL	100.0%
github.com/prebid/prebid-server/v2/adapters/ownadx/ownadx.go:66:	getImpressionExt	100.0%
github.com/prebid/prebid-server/v2/adapters/ownadx/ownadx.go:84:	MakeRequests		93.8%
github.com/prebid/prebid-server/v2/adapters/ownadx/ownadx.go:111:	groupImpsByExt		100.0%
github.com/prebid/prebid-server/v2/adapters/ownadx/ownadx.go:126:	MakeBids		100.0%
github.com/prebid/prebid-server/v2/adapters/ownadx/ownadx.go:190:	Builder			80.0%
github.com/prebid/prebid-server/v2/adapters/ownadx/ownadx.go:203:	getMediaType		66.7%
total:									(statements)		92.8%

@SyntaxNode SyntaxNode changed the title Resolve issue #3774 ownadx: Resolve issue #3774 Jul 24, 2024
@github-actions GitHub Actions
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, 684d42e

ownadx

Refer here for heat map coverage report

github.com/prebid/prebid-server/v2/adapters/ownadx/ownadx.go:24:	getRequestData		83.3%
github.com/prebid/prebid-server/v2/adapters/ownadx/ownadx.go:52:	createBidRequest	100.0%
github.com/prebid/prebid-server/v2/adapters/ownadx/ownadx.go:57:	buildEndpointURL	100.0%
github.com/prebid/prebid-server/v2/adapters/ownadx/ownadx.go:66:	getImpressionExt	100.0%
github.com/prebid/prebid-server/v2/adapters/ownadx/ownadx.go:84:	MakeRequests		93.8%
github.com/prebid/prebid-server/v2/adapters/ownadx/ownadx.go:111:	groupImpsByExt		100.0%
github.com/prebid/prebid-server/v2/adapters/ownadx/ownadx.go:126:	MakeBids		100.0%
github.com/prebid/prebid-server/v2/adapters/ownadx/ownadx.go:190:	Builder			80.0%
github.com/prebid/prebid-server/v2/adapters/ownadx/ownadx.go:203:	getMediaType		66.7%
total:									(statements)		92.8%

onkarvhanumante
onkarvhanumante reviewed Aug 6, 2024
View reviewed changes
static/bidder-info/ownadx.yaml
endpoint: "https://pbs.prebid-ownadx.com/bidder/bid/{{.AccountID}}/{{.ZoneID}}?token={{.SourceId}}"
endpoint: "https://pbs.prebid-ownadx.com/bidder/bid/{{.SeatID}}/{{.SspID}}?token={{.TokenID}}"
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible to pass SeatID SspID as query param?

Consider that usage of partial dynamic urls is being discouraged and prebid team in future will be working to avoid usage of dynamic subdomain in endpoint url.
Major concerns with such usage are,

  • security concerns
    The security aspect is alleviated by using a fixed top level domain. Due to the potential harm to hosts, we are strict in this requirement. We are working towards fixing the few adapters currently in violation.

  • connection performance
    The connection performance advice is for your benefit. Client specific subdomains prevent Prebid Server from reusing connections across your clients which results in more connections needed for your adapter. The issue gets worse the more successful you are in attracting new clients. Hosts may choose to disable your adapter due to this behavior.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Describe in issue #3774.
We added changes accordingly.
This is our standard endpoint for all client, if we change it its will empact on all other clients also.

@onkarvhanumante
Copy link
Contributor

@ownAdx-prebid

PR checks are failing due formatting errors in macros.go. Run go fmt to fix it.

Run ./validate.sh --nofmt --cov --race 10
gofmt needs to be run, 1 files have issues.  Below is a list of files to review:
macros/macros.go

@bsardo bsardo changed the title ownadx: Resolve issue #3774 OwnAdx: Bidder param and URL updates Aug 6, 2024
@github-actions GitHub Actions
Copy link

github-actions bot commented Aug 7, 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, 6e7dc96

ownadx

Refer here for heat map coverage report

github.com/prebid/prebid-server/v2/adapters/ownadx/ownadx.go:24:	getRequestData		83.3%
github.com/prebid/prebid-server/v2/adapters/ownadx/ownadx.go:52:	createBidRequest	100.0%
github.com/prebid/prebid-server/v2/adapters/ownadx/ownadx.go:57:	buildEndpointURL	100.0%
github.com/prebid/prebid-server/v2/adapters/ownadx/ownadx.go:66:	getImpressionExt	100.0%
github.com/prebid/prebid-server/v2/adapters/ownadx/ownadx.go:84:	MakeRequests		93.8%
github.com/prebid/prebid-server/v2/adapters/ownadx/ownadx.go:111:	groupImpsByExt		100.0%
github.com/prebid/prebid-server/v2/adapters/ownadx/ownadx.go:126:	MakeBids		100.0%
github.com/prebid/prebid-server/v2/adapters/ownadx/ownadx.go:190:	Builder			80.0%
github.com/prebid/prebid-server/v2/adapters/ownadx/ownadx.go:203:	getMediaType		66.7%
total:									(statements)		92.8%

@github-actions GitHub Actions
Copy link

github-actions bot commented Aug 7, 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, 97ea498

ownadx

Refer here for heat map coverage report

github.com/prebid/prebid-server/v2/adapters/ownadx/ownadx.go:24:	getRequestData		83.3%
github.com/prebid/prebid-server/v2/adapters/ownadx/ownadx.go:52:	createBidRequest	100.0%
github.com/prebid/prebid-server/v2/adapters/ownadx/ownadx.go:57:	buildEndpointURL	100.0%
github.com/prebid/prebid-server/v2/adapters/ownadx/ownadx.go:66:	getImpressionExt	100.0%
github.com/prebid/prebid-server/v2/adapters/ownadx/ownadx.go:84:	MakeRequests		93.8%
github.com/prebid/prebid-server/v2/adapters/ownadx/ownadx.go:111:	groupImpsByExt		100.0%
github.com/prebid/prebid-server/v2/adapters/ownadx/ownadx.go:126:	MakeBids		100.0%
github.com/prebid/prebid-server/v2/adapters/ownadx/ownadx.go:190:	Builder			80.0%
github.com/prebid/prebid-server/v2/adapters/ownadx/ownadx.go:203:	getMediaType		66.7%
total:									(statements)		92.8%

@gargcreation1992 gargcreation1992 merged commit 4d64623 into prebid:master Aug 12, 2024
5 checks passed
@cross-repo-issue-creation cross-repo-issue-creation bot mentioned this pull request Aug 12, 2024
Closed
bevenio pushed a commit to tamedia-adtec/prebid-server that referenced this pull request Aug 22, 2024
@ownAdx-prebid @bevenio
OwnAdx: Bidder param and URL updates (prebid#3813)
00ae1ac 
Co-authored-by: Hina Yadav <hina.yadav@vertoz.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SyntaxNode SyntaxNode SyntaxNode left review comments

@gargcreation1992 gargcreation1992 gargcreation1992 approved these changes

@onkarvhanumante onkarvhanumante onkarvhanumante approved these changes

Assignees

@gargcreation1992 gargcreation1992

@onkarvhanumante onkarvhanumante

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ownAdx-prebid @onkarvhanumante @SyntaxNode @gargcreation1992