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: Connektai - Xeworks alias #3730 #3738

Merged
merged 5 commits into from
Feb 20, 2025

Conversation

przemkaczmarek
Copy link
Collaborator

@przemkaczmarek przemkaczmarek commented Feb 7, 2025

🔧 Type of changes

  • new bid adapter
  • [ ]

✨ What's the context?

#3730

Comment on lines 1 to 20
adapters:
connektai:
endpoint: http://rtb.connektai.live/?pid={{.SourceId}}&host={{.Host}}&s=pbs
aliases:
xeworks:
meta-info:
maintainer-email: adops@connekt.ai
app-media-types:
- banner
- video
- native
site-media-types:
- banner
- video
- native
supported-vendors:
vendor-id: 0
usersync:
cookie-family-name: xeworks
redirect:
Copy link
Collaborator

Choose a reason for hiding this comment

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

this config won't work, please make it properly + add the integration test for the new alias

P.S. the placeholders won't work as well

AntoxaAntoxic
AntoxaAntoxic previously approved these changes Feb 13, 2025
@@ -500,6 +500,8 @@ adapters.vrtcal.enabled=true
adapters.vrtcal.endpoint=http://localhost:8090/vrtcal-exchange
adapters.xeworks.enabled=true
adapters.xeworks.endpoint=http://localhost:8090/xeworks-exchange
adapters.xeworks.aliases.connektai.enabled=true
adapters.xeworks.aliases.connektai.endpoint=http://localhost:8090/connektai-exchange?host={{Host}}&sourceId={{SourceId}}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove params here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I added params because:
image

Comment on lines 23 to 24
.withQueryParam("host", equalTo("envValue"))
.withQueryParam("sourceId", equalTo("pidValue"))
Copy link
Collaborator

Choose a reason for hiding this comment

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

There is no need to test this. It is already tested in the unit tests for the base bidder. We write integration tests only to check that the configurations are correct and that the simplest request will pass from request to response.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I added this because @AntoxaAntoxic said to do it.
Okey, I understand. So where should I check if, for example, in a YAML file, we have two fields, but only one is required, such as placementId? Should I write the tests here?

Copy link
Member

@And1sS And1sS left a comment

Choose a reason for hiding this comment

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

+1 for @CTMBNara s comment. We don't test config in IT tests. Please, remove params as Danylo suggested.

@CTMBNara CTMBNara merged commit b5a9b28 into master Feb 20, 2025
8 checks passed
@CTMBNara CTMBNara deleted the New-adapter-Connektai-Xeworks-alias-#3730 branch February 20, 2025 10:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Port PR from PBS-Go: New adapter: Connektai - Xeworks alias
5 participants