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

Rubicon Adapter - SRA support #2478

Merged
merged 6 commits into from
May 23, 2018

Conversation

idettman
Copy link
Contributor

@idettman idettman commented May 2, 2018

Type of change

  • Feature

Description of change

Adds "Single Request Architecture" (SRA) support to the Rubicon Adapter.

// By default, SRA mode is turned off

// Enable SRA with the following config change
pbjs.setConfig({
  rubicon: {
    singleRequest: true
  }
});
  • test parameters for validating bids
const exampleBids = [{
    code: 'div-1', sizes: [[728, 90], [970, 90]], bids: [{
        bidder: 'rubicon', params: {
            accountId: "1234", siteId: "1234", zoneId: "1001"
        }
    }, {
        bidder: 'rubicon', params: {
            accountId: "1234", siteId: "1234", zoneId: "1002",
        }
    }, {
        bidder: 'rubicon', params: {
            accountId: "1234", siteId: "9876", // Note: <-- this siteId doesn't match the other two bids, so a second request is sent for bids with that 'siteId' value
            zoneId: "1003"
        }
    }]
}];

Other information

HB-2542

@mike-chowla
Copy link
Contributor

@idettman There's a merge conflict. Can you resolve it?

@mike-chowla
Copy link
Contributor

The test parameters for validating bids are missing closing brackets under parameters structure. All three have the missing bracket

I'm not getting bids back with them either

Request:
http://fastlane.rubiconproject.com/a/api/fastlane.json?account_id=1234&site_id=1234&zone_id=1001%3B1002&size_id=2&alt_size_ids=55&p_pos=unknown&rp_floor=0.01&rp_secure=0&tk_flint=pbjs_lite_v1.10.0-pre&x_source.tid=c5ee2f86-7022-4964-9149-92e0bba1ec94&p_screen_res=1920x1080&rf=http%3A%2F%2Fmike.pubmatic.com%3A9999%2FintegrationExamples%2Fgpt%2Fhello_world.html%3Fpbjs_debug%3Dtrue&slots=2&rand=0.5636169652091298

Response:

{  
   "status":"error",
   "reason":"internal-error",
   "error_code":"16",
   "account_id":1234,
   "site_id":1234,
   "zone_id":1001,
   "tracking":"",
   "inventory":{  

   },
   "ads":[  
      {  
         "status":"error",
         "reason":"internal-error",
         "error_code":"16",
         "impression_id":"b6e5b7ce-0131-4dba-aef9-455c7793c421"
      },
      {  
         "status":"error",
         "reason":"internal-error",
         "error_code":"51",
         "impression_id":"bcb0ffff-b4f1-4277-8f19-0651a3904fc5"
      }
   ]
}

Copy link
Contributor

@mike-chowla mike-chowla left a comment

Choose a reason for hiding this comment

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

Please fix the merge conflict.

This change is large enough that it should be validated with working test parameters but I'm not getting bids back on the test params

I did not see any code issues

@idettman idettman closed this May 15, 2018
@idettman idettman reopened this May 17, 2018
@idettman idettman requested a review from snapwich May 17, 2018 19:42
Copy link
Collaborator

@snapwich snapwich left a comment

Choose a reason for hiding this comment

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

LGTM

@idettman
Copy link
Contributor Author

@mike-chowla regarding your comment on working test params: unfortunately our endpoint will not be able to handle SRA requests until after the Prebid release, we're merging with a feature that is off by default, so it shouldn't cause issues with existing since it has to be enabled though config. We have tested internally with integration tests so we're confident the code change isn't a breaking change.

@mike-chowla mike-chowla merged commit 1d5c8ee into prebid:master May 23, 2018
@idettman idettman deleted the rubicon-adapter-sra-rebase branch February 5, 2019 00:03
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.

3 participants