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

Alkimi: Add imp extension support #4213

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

Alkimi: Add imp extension support #4213

wants to merge 111 commits into from
+15 −10

Conversation

kalidas-alkimi
Copy link
Contributor

Added bidder extension as new feature.

pro-nsk and others added 21 commits November 16, 2023 09:13
@pro-nsk
imports fixed
46a764d
@pro-nsk
Merge branch 'master' into master_asterio
be64f7b 
# Conflicts:
#	adapters/alkimi/alkimi_test.go
@pro-nsk
tests fixed
681a034
@pro-nsk
tests fixed
325861d
@pro-nsk
tests fixed
e77d00c
@pro-nsk
remove unnecessary tests
c3fe4cc
@pro-nsk
Merge branch 'master' into master_asterio
39a5b97 
# Conflicts:
#	adapters/alkimi/alkimi_test.go
@pro-nsk
tests fixed
1e6bfdc
@pro-nsk
tests fixed
e90c6d3
@pro-nsk
tests fixed
4541d7e
@pro-nsk
Merge pull request #2 from prebid/master
2d298c2 
merge master
@pro-nsk
Merge branch 'master_asterio' into master
59a1a87
@kalidas-alkimi
Added User sync url for Alkimi adapter
7530153
@pro-nsk
revert changes
4ad7bd0
@pro-nsk
fix user-sync url
da32542
@kalidas-alkimi
Updated the user sync url to prod env instead of dev env
c7e0320
@kalidas-alkimi
Merge branch 'master' of https://github.com/Alkimi-Exchange/prebid-se…
50a999a 
…rver
@pro-nsk
Merge branch 'master' of github.com:prebid/prebid-server into prebid-…
92509c8 
…master2
@pro-nsk
Merge branch 'prebid-master2'
f3d1eac
@pro-nsk
Merge pull request #6 from prebid/master
055866e 
merge master
@kalidas-alkimi
Added support for Imp extention object
9b60ac7
@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, 9b60ac7

alkimi

Refer here for heat map coverage report

github.com/prebid/prebid-server/v3/adapters/alkimi/alkimi.go:28:	Builder			100.0%
github.com/prebid/prebid-server/v3/adapters/alkimi/alkimi.go:41:	MakeRequests		81.8%
github.com/prebid/prebid-server/v3/adapters/alkimi/alkimi.go:60:	updateImps		76.5%
github.com/prebid/prebid-server/v3/adapters/alkimi/alkimi.go:115:	buildBidderRequest	100.0%
github.com/prebid/prebid-server/v3/adapters/alkimi/alkimi.go:131:	MakeBids		96.2%
github.com/prebid/prebid-server/v3/adapters/alkimi/alkimi.go:177:	resolveMacros		100.0%
github.com/prebid/prebid-server/v3/adapters/alkimi/alkimi.go:183:	getMediaTypeForImp	100.0%
total:									(statements)		88.2%

linux019
linux019 suggested changes Feb 14, 2025
View reviewed changes
Copy link
Contributor

@linux019 linux019 left a comment

Choose a reason for hiding this comment

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

Please use the jsonutil package to marshal/unmarshal data everywhere. PBS is in the process of removing standard json, which is slow and makes more memory allocations compared to jsonutil

@bsardo bsardo changed the title Added support for Imp extention object Alkimi: Add imp extension support Feb 18, 2025
@bsardo bsardo added the adapter label Feb 18, 2025
@bsardo
Copy link
Collaborator

bsardo commented Feb 25, 2025

@pm-jaydeep-mohite can you please review?

pm-jaydeep-mohite
pm-jaydeep-mohite reviewed Feb 27, 2025
View reviewed changes
adapters/alkimi/alkimi.go
temp := extObj{AlkimiBidderExt: extImpAlkimi}
temp.AlkimiBidderExt.AdUnitCode = imp.ID
temp := extImpAlkimi
temp.AdUnitCode = imp.ID
Copy link
Contributor

Choose a reason for hiding this comment

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

Is imp.ID required to be part of impression extension as this is standard ORTB parameter ?
Usually bidder specific custom parameters are added in impression extension.

When bidder receive request, imp.ID can be used as AdUnitCode

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it's required for bidder.
But this is not related to this PR, in this PR we've added support of any parameters in imp.ext map.

@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, 9406a5c

alkimi

Refer here for heat map coverage report

github.com/prebid/prebid-server/v3/adapters/alkimi/alkimi.go:28:	Builder			100.0%
github.com/prebid/prebid-server/v3/adapters/alkimi/alkimi.go:41:	MakeRequests		81.8%
github.com/prebid/prebid-server/v3/adapters/alkimi/alkimi.go:60:	updateImps		76.5%
github.com/prebid/prebid-server/v3/adapters/alkimi/alkimi.go:115:	buildBidderRequest	100.0%
github.com/prebid/prebid-server/v3/adapters/alkimi/alkimi.go:131:	MakeBids		96.2%
github.com/prebid/prebid-server/v3/adapters/alkimi/alkimi.go:177:	resolveMacros		100.0%
github.com/prebid/prebid-server/v3/adapters/alkimi/alkimi.go:183:	getMediaTypeForImp	100.0%
total:									(statements)		88.2%

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pm-jaydeep-mohite pm-jaydeep-mohite pm-jaydeep-mohite left review comments

@linux019 linux019 linux019 requested changes

At least 2 approving reviews are required to merge this pull request.

Assignees

@guscarreon guscarreon

Labels
adapter
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@kalidas-alkimi @bsardo @linux019 @pm-jaydeep-mohite @guscarreon @abalalaev @pro-nsk