-
Notifications
You must be signed in to change notification settings - Fork 802
JSON block that has a full data-center specific URL cache info #1104
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
exchange/exchange.go
Outdated
var found bool = false | ||
|
||
if auc != nil { | ||
if bid.bidType == openrtb_ext.BidTypeVideo { |
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.
There may be use cases in the future for video bids to get a full bid cached, or non-video bids to get some sort of vast cache. Also we cannot 100% trust adapters to report the bidType
correctly. We should just check for a bid cache first, and if that does not exist, check for a VAST cache.
exchange/exchange.go
Outdated
|
||
if found { | ||
cacheInfo.CacheId = cacheUUID | ||
cacheInfo.Url = e.cache.GetPutUrl() + "?uuid=" + cacheUUID //if there's no UUID, should we still define a url? |
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.
Why are you calling this method GetPutUrl()
? Wouldn't be GetGetUrl()
, as it is the URL that clients will use to issue the GET request to get the cached content?
prebid_cache_client/client.go
Outdated
@@ -72,6 +75,10 @@ func (c *clientImpl) GetExtCacheData() (string, string) { | |||
return c.externalCacheHost, c.externalCachePath | |||
} | |||
|
|||
func (c *clientImpl) GetPutUrl() string { |
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.
For this PR, we want to use the URL we can assemble from getExtCacheData()
, we don't need a new method. When we return a URL to a client, we always want to return an external URL, not an PBS internal URLs, as they will be useless to the client if the internal differs from the external.
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.
See inline comments ... we want the external URL, not internal URL.
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
No description provided.