-
Notifications
You must be signed in to change notification settings - Fork 207
Addigi: New alias for Smarthub #3848
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it a port from Go? Or should it be ported to Go?
it's a port from Go: #3750 |
@@ -457,6 +457,8 @@ adapters.smarthub.aliases.jambojar.enabled=true | |||
adapters.smarthub.aliases.jambojar.endpoint=http://localhost:8090/jambojar-exchange?host={{Host}}&accountId={{AccountID}}&sourceId={{SourceId}} | |||
adapters.smarthub.aliases.adinify.enabled=true | |||
adapters.smarthub.aliases.adinify.endpoint=http://localhost:8090/adinify-exchange?host={{Host}}&accountId={{AccountID}}&sourceId={{SourceId}} | |||
adapters.smarthub.aliases.addigi.enabled=true | |||
adapters.smarthub.aliases.addigi.endpoint=http://localhost:8090/addigi-exchange?host={{Host}}&accountId={{AccountID}}&sourceId={{SourceId}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
placeholders are unnecessary if they are not checked in the test
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hi sorry if this is a silly question, does this not count as being checked in the test? post(urlPathEqualTo("/addigi-exchange"))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it mocks all the requests going to the path /addigi-exchange
neglecting query params
so from my perspective having query params in the http://localhost:8090/addigi-exchange?host={{Host}}&accountId={{AccountID}}&sourceId={{SourceId}}
is kind of redundant, because it doesn't impact the test at all
therefore I can suggest using just the http://localhost:8090/addigi-exchange
endpoint which is sufficient OR as an alternative you can add checks for query params in the test
🔧 Type of changes
✨ What's the context?
What's the context for the changes?
Addigi alias added to SmartHub (Attekmi)
🧠 Rationale behind the change
Why did you choose to make these changes? Were there any trade-offs you had to consider?
N/A - just an alias
🔎 New Bid Adapter Checklist
🧪 Test plan
How do you know the changes are safe to ship to production?
🏎 Quality check