Remove /auction Endpoint #2033

SyntaxNode · 2021-10-05T20:04:07Z

!! Do not merge until the release after #2032 !!

Implements phase 3 of #2011. I ran go-lint to find and remove now unused code, and found a few unrelated dead code references which I removed here too.

SyntaxNode · 2021-10-05T20:05:00Z

stored_requests/events/events.go

@@ -77,7 +77,7 @@ func (e *EventListener) Listen(cache stored_requests.Cache, events EventProducer
 				e.onInvalidate()
 			}
 		case <-e.stop:
-			break
+			return


Fixed an unrelated bug. :)

guscarreon

Left a couple of nitpicks. Also, the pbs/usersync.go file that contains in the "/optout" endpoint functionality was not removed, can it be relocated to, say endpoints/output.go in a future PR? Or maybe even renamed to captcha/usersync.go or something.

guscarreon · 2021-10-12T14:18:59Z

adapters/ix/ix.go

@@ -454,14 +222,6 @@ func (a *IxAdapter) MakeBids(internalRequest *openrtb2.BidRequest, externalReque
 	return bidderResponse, errs
 }

-func NewIxLegacyAdapter(config *adapters.HTTPAdapterConfig, endpoint string) *IxAdapter {


This wasn't even used. Good catch.

guscarreon · 2021-10-12T14:28:48Z

adapters/ix/ix_test.go

@@ -31,698 +24,6 @@ func TestJsonSamples(t *testing.T) {
 	}
 }

-// Tests for the legacy, non-openrtb code.
-// They can be removed after the legacy interface is deprecated.


Impressed how many lines of code we don't need anymore.

Agreed. Very satisfying.

guscarreon · 2021-10-12T15:08:46Z

router/router.go

@@ -49,16 +31,15 @@ import (
 	storedRequestsConf "github.com/prebid/prebid-server/stored_requests/config"
 	"github.com/prebid/prebid-server/usersync"
 	"github.com/prebid/prebid-server/util/sliceutil"
+	"github.com/prebid/prebid-server/util/uuidutil"
+	"github.com/prebid/prebid-server/version"

 	"github.com/golang/glog"
 	"github.com/julienschmidt/httprouter"
 	_ "github.com/lib/pq"


We can probably remove this one too.

I'm not sure. This is initializing the postgres package which we still use for stored requests. I feel more comfortable experimenting with this in a future PR.

Actually, this looks like it's still necessary since it's registering the postgres db driver via the package's init method.

FYI: I personally dislike Go init methods.

guscarreon · 2021-10-12T15:28:21Z

stored_requests/config/config.go

@@ -107,7 +107,7 @@ func CreateStoredRequests(cfg *config.StoredRequests, metricsEngine metrics.Metr
 //
 // As a side-effect, it will add some endpoints to the router if the config calls for it.
 // In the future we should look for ways to simplify this so that it's not doing two things.
-func NewStoredRequests(cfg *config.Configuration, metricsEngine metrics.MetricsEngine, client *http.Client, router *httprouter.Router) (db *sql.DB, shutdown func(), fetcher stored_requests.Fetcher, ampFetcher stored_requests.Fetcher, accountsFetcher stored_requests.AccountFetcher, categoriesFetcher stored_requests.CategoryFetcher, videoFetcher stored_requests.Fetcher) {
+func NewStoredRequests(cfg *config.Configuration, metricsEngine metrics.MetricsEngine, client *http.Client, router *httprouter.Router) (shutdown func(), fetcher stored_requests.Fetcher, ampFetcher stored_requests.Fetcher, accountsFetcher stored_requests.AccountFetcher, categoriesFetcher stored_requests.CategoryFetcher, videoFetcher stored_requests.Fetcher) {


Can we remove DB connection entry that is no longer returned by this function from the comment above?

96 // NewStoredRequests returns five things: 97 // 98 - // 1. A DB connection, if one was created. This may be nil. 99 // 2. A function which should be called on shutdown for graceful cleanups. 100 // 3. A Fetcher which can be used to get Stored Requests for /openrtb2/auction 101 // 4. A Fetcher which can be used to get Stored Requests for /openrtb2/amp 102 // 5. A Fetcher which can be used to get Category Mapping data 103 // 6. A Fetcher which can be used to get Stored Requests for /openrtb2/video 104 // 105 // If any errors occur, the program will exit with an error message. 106 // It probably means you have a bad config or networking issue. 107 // 108 // As a side-effect, it will add some endpoints to the router if the config calls for it. 109 // In the future we should look for ways to simplify this so that it's not doing two things. 110 func NewStoredRequests(cfg *config.Configuration, metricsEngine metrics.MetricsEngine, client *http.Client, router *httprouter.Router) (shutdown func(), fetcher stored_requests.Fetcher, ampFetcher store

Yes. I'll update the comment. Good catch.

guscarreon

LGTM

braizhas · 2021-10-12T22:35:24Z

@SyntaxNode submitted PR SyntaxNode#3 making adform adapter as adf adapter alias. Is such change acceptable here? Should I create separate PR after this one gets merged?

pm-viral-vala · 2021-10-13T12:23:23Z

adapters/pubmatic/pubmatic_test.go

-func TestGetBidTypeVideo(t *testing.T) {
-	pubmaticExt := new(pubmaticBidExt)
-	pubmaticExt.BidType = new(int)
-	*pubmaticExt.BidType = 1
-	actualBidTypeValue := getBidType(pubmaticExt)
-	if actualBidTypeValue != openrtb_ext.BidTypeVideo {
-		t.Errorf("Expected Bid Type value was: %v, actual value is: %v", openrtb_ext.BidTypeVideo, actualBidTypeValue)
-	}
-}
-
-func TestGetBidTypeForMissingBidTypeExt(t *testing.T) {
-	pubmaticExt := pubmaticBidExt{}
-	actualBidTypeValue := getBidType(&pubmaticExt)
-	// banner is the default bid type when no bidType key is present in the bid.ext
-	if actualBidTypeValue != "banner" {
-		t.Errorf("Expected Bid Type value was: banner, actual value is: %v", actualBidTypeValue)
-	}
-}
-
-func TestGetBidTypeBanner(t *testing.T) {
-	pubmaticExt := new(pubmaticBidExt)
-	pubmaticExt.BidType = new(int)
-	*pubmaticExt.BidType = 0
-	actualBidTypeValue := getBidType(pubmaticExt)
-	if actualBidTypeValue != openrtb_ext.BidTypeBanner {
-		t.Errorf("Expected Bid Type value was: %v, actual value is: %v", openrtb_ext.BidTypeBanner, actualBidTypeValue)
-	}
-}
-
-func TestGetBidTypeNative(t *testing.T) {
-	pubmaticExt := new(pubmaticBidExt)
-	pubmaticExt.BidType = new(int)
-	*pubmaticExt.BidType = 2
-	actualBidTypeValue := getBidType(pubmaticExt)
-	if actualBidTypeValue != openrtb_ext.BidTypeNative {
-		t.Errorf("Expected Bid Type value was: %v, actual value is: %v", openrtb_ext.BidTypeNative, actualBidTypeValue)
-	}
-}
-
-func TestGetBidTypeForUnsupportedCode(t *testing.T) {
-	pubmaticExt := new(pubmaticBidExt)
-	pubmaticExt.BidType = new(int)
-	*pubmaticExt.BidType = 99
-	actualBidTypeValue := getBidType(pubmaticExt)
-	if actualBidTypeValue != openrtb_ext.BidTypeBanner {
-		t.Errorf("Expected Bid Type value was: %v, actual value is: %v", openrtb_ext.BidTypeBanner, actualBidTypeValue)
-	}


Can you please add these required test cases?

Thank you for reviewing the PR. Good catch. These tests have been restored.

SyntaxNode · 2021-10-14T17:05:02Z

@SyntaxNode submitted PR SyntaxNode#3 making adform adapter as adf adapter alias. Is such change acceptable here? Should I create separate PR after this one gets merged?

I recommend creating a separate PR after this one gets merged. This PR is already large with 10k+ lines affected.

guscarreon

LGTM

This reverts commit fe211a9.

SyntaxNode · 2021-10-18T17:45:54Z

This was accidentally merged earlier than intended. Please see: #2049

This reverts commit fe211a9.

Remove /auction Endpoint

4c82909

SyntaxNode commented Oct 5, 2021

View reviewed changes

SyntaxNode added 3 commits October 5, 2021 17:41

Removed Unused Code

741e876

Go Mod Tidy

5d7f89b

Prune Config

fd51784

mansinahar requested review from bsardo and VeronikaSolovei9 October 6, 2021 17:25

mansinahar assigned bsardo and VeronikaSolovei9 Oct 6, 2021

mansinahar requested a review from guscarreon October 11, 2021 17:26

mansinahar assigned guscarreon Oct 11, 2021

guscarreon reviewed Oct 12, 2021

View reviewed changes

SyntaxNode added 2 commits October 12, 2021 13:21

Merge branch 'master' into remove_legacy_endpoint

1a83208

Update Comment

e5858a0

guscarreon previously approved these changes Oct 12, 2021

View reviewed changes

pm-viral-vala reviewed Oct 13, 2021

View reviewed changes

mansinahar unassigned bsardo Oct 13, 2021

mansinahar removed the request for review from bsardo October 13, 2021 17:12

VeronikaSolovei9 previously approved these changes Oct 14, 2021

View reviewed changes

Restored Pubmatic getBidType Tests

19017e1

SyntaxNode dismissed stale reviews from VeronikaSolovei9 and guscarreon via 19017e1 October 14, 2021 17:14

VeronikaSolovei9 approved these changes Oct 14, 2021

View reviewed changes

guscarreon approved these changes Oct 15, 2021

View reviewed changes

mansinahar merged commit fe211a9 into prebid:master Oct 18, 2021

mansinahar added a commit that referenced this pull request Oct 18, 2021

Revert "Remove /auction Endpoint (#2033)"

1dcac5a

This reverts commit fe211a9.

mansinahar mentioned this pull request Oct 18, 2021

Revert "Remove /auction Endpoint" #2048

Merged

SyntaxNode pushed a commit that referenced this pull request Oct 18, 2021

Revert "Remove /auction Endpoint (#2033)" (#2048)

08488d2

This reverts commit fe211a9.

SyntaxNode mentioned this pull request Oct 18, 2021

Remove /auction Endpoint #2049

Merged

jorgeluisrocha pushed a commit to jwplayer/prebid-server that referenced this pull request Sep 28, 2022

Remove /auction Endpoint (prebid#2033)

90d5fa6

jorgeluisrocha pushed a commit to jwplayer/prebid-server that referenced this pull request Sep 28, 2022

Revert "Remove /auction Endpoint (prebid#2033)" (prebid#2048)

a1aa977

This reverts commit fe211a9.

shunj-nb pushed a commit to ParticleMedia/prebid-server that referenced this pull request Nov 8, 2022

Remove /auction Endpoint (prebid#2033)

de70b5a

shunj-nb pushed a commit to ParticleMedia/prebid-server that referenced this pull request Nov 8, 2022

Revert "Remove /auction Endpoint (prebid#2033)" (prebid#2048)

bcb51b9

This reverts commit fe211a9.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove /auction Endpoint #2033

Remove /auction Endpoint #2033

SyntaxNode commented Oct 5, 2021 •

edited

Loading

SyntaxNode Oct 5, 2021

guscarreon left a comment

guscarreon Oct 12, 2021

guscarreon Oct 12, 2021

SyntaxNode Oct 12, 2021

guscarreon Oct 12, 2021

SyntaxNode Oct 12, 2021

SyntaxNode Oct 12, 2021

guscarreon Oct 12, 2021

SyntaxNode Oct 12, 2021

guscarreon left a comment

braizhas commented Oct 12, 2021

pm-viral-vala Oct 13, 2021

SyntaxNode Oct 14, 2021

SyntaxNode commented Oct 14, 2021

guscarreon left a comment

SyntaxNode commented Oct 18, 2021

Remove /auction Endpoint #2033

Remove /auction Endpoint #2033

Conversation

SyntaxNode commented Oct 5, 2021 • edited Loading

Choose a reason for hiding this comment

guscarreon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

guscarreon left a comment

Choose a reason for hiding this comment

braizhas commented Oct 12, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SyntaxNode commented Oct 14, 2021

guscarreon left a comment

Choose a reason for hiding this comment

SyntaxNode commented Oct 18, 2021

SyntaxNode commented Oct 5, 2021 •

edited

Loading