Decision #30

abushev · 2017-05-15T21:16:35Z

No description provided.

…r to pass extended requests

venediktov · 2017-05-18T04:25:47Z

examples/bidder/http_bidder_test.cpp

@@ -42,13 +46,15 @@ auto random_pick(int max) {
  return dis(gen);
 }

-
+class http_bidder_decisions_manager {


not used , can be removed.

venediktov · 2017-05-18T04:28:15Z

examples/bidder/http_bidder_test.cpp

 int main(int argc, char *argv[]) {
    using namespace std::placeholders;
    using namespace vanilla::exchange;
    using namespace std::chrono_literals;
    using restful_dispatcher_t =  http::crud::crud_dispatcher<http::server::request, http::server::reply> ;
-    using BidRequest = openrtb::BidRequest<std::string>;
+    //using BidRequest = openrtb::BidRequest<std::string>;


Should we stick to original idea DSL::serialized and DSL::deserialized ?
See the use case in exchange_handler_test.cpp

using DSLT = DSL::GenericDSL<std::string> ; using BidRequest = DSLT::deserialized_type; using BidResponse = DSLT::serialized_type;

This will eliminate a need to use exchange_hander::types_blah_blah_blah

venediktov · 2017-05-18T04:43:56Z

examples/bidder/http_bidder_test.cpp

+    };
+    const decision_exchange_type::decision_tree_type decision_tree = {{
+        {static_cast<int> (decision_codes_type::USER_DATA),
+            {request_user_data_f, static_cast<int> (decision_codes_type::AUCTION_ASYNC), static_cast<int> (decision_codes_type::NO_BID),}},


extra comma after NO_BID)** ,**

venediktov · 2017-05-18T04:44:51Z

examples/bidder/http_bidder_test.cpp

+    auto auction_async_f = [&bid_handler](http::server::reply &reply, BidRequest & bid_request) -> bool {
+        return bid_handler.handle_auction_async(reply, bid_request);
+    };
+    const decision_exchange_type::decision_tree_type decision_tree = {{


How about this format for readability ?

const decision_exchange_type::decision_tree_type decision_tree = {{ { static_cast<int> (decision_codes_type::USER_DATA), {request_user_data_f, static_cast<int> (decision_codes_type::AUCTION_ASYNC), static_cast<int> (decision_codes_type::NO_BID)} }, { static_cast<int> (decision_codes_type::NO_BID), {no_bid_f, decision_exchange_type::decision_action::EXIT, decision_exchange_type::decision_action::EXIT} }, { static_cast<int> (decision_codes_type::AUCTION_ASYNC), {auction_async_f, decision_exchange_type::decision_action::EXIT, decision_exchange_type::decision_action::EXIT} } }};

Seems that It's not easy from this to define from the first look what are the output routes.
I'll try little bit different format.
I want to give decision tree user ability to define enum for decisions just in place it meant to be used, without additional namespaces.
But tree record with enum class seems redundant, too long etc.

The sample code below is more readable because it's very compact, and yes I agree it's hard to see the output routes, so formatting definitely helps to improve readability even with enums.

decision_tree_manager<3> tm = decision_tree<3> {{ { 0 , { action_1, -1 , 1} }, //true->EXIT , false->index=1 { 1 , { action_2, -1 , 2} }, //true->EXIT , false->index=2 { 2 , { action_3, -1 , -1} } //EXIT }} ;

venediktov · 2017-05-18T05:04:55Z

examples/bidder/http_bidder_test.cpp

-            }
-            return response;
-        });
+            thread_local vanilla::ResponseBuilder<BidderConfig> response_builder(caches);


Should we rename ResponseBuilder to Bidder ? Because it uses cache selector and filters and creates response. From reading this part of code it's hard to tell where the actual bidding occurs.
So this could be potential our bidder class?

thread_local vanilla::Bidder<BidderConfig> bidder(caches); return bidder.process(request);

I think its good idea

venediktov · 2017-05-18T05:38:53Z

examples/bidder/http_bidder_test.cpp

-    exchange_handler<DSL::GenericDSL<>> bid_handler(std::chrono::milliseconds(config.data().timeout));
+
+
+    using bid_handler_type = exchange_handler<DSL::GenericDSL<>, vanilla::VanillaRequest>;


I see now what's going on now ... the exchange_handler was designed to handle request in the generic way relying on DSL::serialized_type , DSL::deserialized_type , if we move it to vanilla::VanillaRequest then we'll need to maintain conversions for all different DSL types inside of
core/bid_request.hpp .
It basically breaks our model ?
I also see that user matching relies on vanilla::VanillaRequest as it has user_info.
We need to think about it.

Year the problem in in vanilla::BidRequest and VanillaRequest.
They are not flexible

venediktov · 2017-05-18T05:57:17Z

we have to think about openrtb::BidRequest -> vanilla::BidRequest , perhaps vanilla::BidRequest needs to be generic wrapper around any BidRequest type , and in this case we can pass it as

template<typename DSL , template<class> class  InfoEnricher>
exchange_handler {
using auction_request_type = InfoEnricher<DSL::deserialized_type>;
...
...
};

Something like this ?

namespace vanilla {
    template <typename UserInfo, typename ExternalBidRequest>
    struct BidRequest {
        ExternalBidRequest bid_request;
        UserInfo user_info;
    };
}

abushev · 2017-05-18T21:40:44Z

I'm trying to make Bidder class, vanilla::BidRequest and others more flexible

abushev · 2017-05-19T21:28:57Z

Made some changes to make Bidder, multibidder_communicator and other more flexible.
Not sure about using template template arguments in multibidder_communicator and
exchange_handler.

In exchagne_handler if I dont want to extend handling to auction data then
I still can use default parameter, and dont need InfoEnricher.
Yeah, I think we need to discuss this.

venediktov · 2017-05-20T17:13:40Z

rtb/exchange/multibidder_communicator.hpp

    public:
        multibidder_communicator(uint16_t bidders_port, Duration response_timeout) :
            response_timeout(response_timeout) {
            communicator.outbound(bidders_port);
        }

-        template<typename T>
-        void process(const vanilla::VanillaRequest &vanilla_request, multibidder_collector<T> &collector) {
+        template<typename Request>


I think we can use any collector here ?

template<typename Request, typename Collector> void process(const Request &request, Collector &collector) {

I would also suggest removing depencies on very nested types
DSL::serialized_type::data_type , and even further maybe in future replace
DSL::deserialized_type -> DSL::request_type
DSL::serialized_type -> DSL::response_type

Because I sometimes forget which one is which :-)

venediktov · 2017-05-20T23:01:30Z

good work Arseny, let's push and see if we need to adjust it !

venediktov · 2017-05-20T23:22:01Z

finally we have decision_router based on decision_tree to build our pipeline !!!!

abushev added 11 commits May 1, 2017 12:02

decisions

3d3e7d3

bug fix

4e4ea5e

Merge branch 'master' into decision

777a470

Merge branch 'master' into decision

c5b54df

decision in progress, added Request template param to exchange_handle…

8c55c90

…r to pass extended requests

decisions

8fbb2b7

refactoring decision making

91f21e6

decisions

a18ea5c

decision tree for actions

129dab4

variadic template and matched user data request example

58eb690

code style and some refactoring

6f91c93

venediktov requested changes May 18, 2017

View reviewed changes

some requested changes added

3a3e976

flexibility

7236df0

venediktov reviewed May 20, 2017

View reviewed changes

venediktov merged commit ac21ea5 into master May 20, 2017

venediktov deleted the decision branch June 26, 2017 21:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Decision #30

Decision #30

abushev commented May 15, 2017

venediktov May 18, 2017

venediktov May 18, 2017

venediktov May 18, 2017

venediktov May 18, 2017

abushev May 18, 2017

venediktov May 19, 2017

venediktov May 18, 2017

abushev May 18, 2017

venediktov May 18, 2017

abushev May 18, 2017

venediktov commented May 18, 2017 •

edited

abushev commented May 18, 2017 •

edited

abushev commented May 19, 2017

venediktov May 20, 2017

venediktov commented May 20, 2017

venediktov commented May 20, 2017

		exchange_handler<DSL::GenericDSL<>> bid_handler(std::chrono::milliseconds(config.data().timeout));


		using bid_handler_type = exchange_handler<DSL::GenericDSL<>, vanilla::VanillaRequest>;

Decision #30

Decision #30

Conversation

abushev commented May 15, 2017

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

Choose a reason for hiding this comment

venediktov commented May 18, 2017 • edited

abushev commented May 18, 2017 • edited

abushev commented May 19, 2017

Choose a reason for hiding this comment

venediktov commented May 20, 2017

venediktov commented May 20, 2017

venediktov commented May 18, 2017 •

edited

abushev commented May 18, 2017 •

edited