----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39694/#review104505 -----------------------------------------------------------
I have what I think is a simpler implementation of connection options, I can't update the reviewboard look at: https://github.com/alanconway/qpid-proton/commit/7eb1e87c1fdc8a72422b36770acbe0d3fa2deb1c The logic is Cliff's (if I didn't miss anything) but the impl is shorter and I think more readable. If we like this we could do the same for link options. NO-JIRA: Simpler implementation of connection options. This is a simpler implementation of connection_options - More compact, option tests and apply logic together in one apply() function. - No limit on number of options that can be chained in call to connect() - Simple value semantics. No need for virtual clone(), inheritance, pointers. - Fewer heap allocations - one per option set, not per individual option. Options class is pimpl with no virtuals so no binary compat issues adding new options. /** Options for creating a connection. * * Options can be "chained" like this: * * c = container.connect(url, connection_options().handler(h).max_frame_size(1234)); * * You can also create an options object with common settings and use it as a base * for different connections that have mostly the same settings: * * connection_options opts; * opts.idle_timeout(1000).max_frame_size(10000); * c1 = container.connect(url1, opts.handler(h1)); * c2 = container.connect(url2, opts.handler(h2)); * * Normal value semantics, copy or assign creates a separate copy of the options. */ - Alan Conway On Oct. 27, 2015, 6:21 p.m., Cliff Jansen wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/39694/ > ----------------------------------------------------------- > > (Updated Oct. 27, 2015, 6:21 p.m.) > > > Review request for qpid, Alan Conway, Andrew Stitcher, and Justin Ross. > > > Bugs: PROTON-865 > https://issues.apache.org/jira/browse/PROTON-865 > > > Repository: qpid-proton-git > > > Description > ------- > > This provides connection options (broadly similar to link options). > > As suggested by Alan, the collection connection_options is a subclass of the > base connection_option so that they can be grouped and nested. > > They can also be copied and persisted via respective clone methods. > Reconnect and transport options need to live as long as the connection. The > handler option is special and needs to be extracted before other options to > be available when the connection is created. > > Incoming (listener) connection options are perhaps missing a feature. You > can set global server connection options on a container, and you can set > separate ssl_domain creds per listener, but you cannot set other connection > options (idle_timeout/max_frame_size etc) per listener. This is reflected in > Proton-C's acceptor and the readable events that it intercepts. There is no > way to query the C API to find out which listener is the parent of an > incoming connection or transfer other listener-specific context to the > connection. > > As suggested in the link options review, I attempted to provide an > initializer_list<foo> constructor and append, but that requires array > elements of identical size. The varying size of the connection option types > is unhelpful here. Maybe they should be counted_ptr based pimpl structs of > identical size. > > This patch can also be viewed as the "reconnect" branch of > https://github.com/cliffjansen/qpid-proton. > > > Diffs > ----- > > examples/cpp/CMakeLists.txt 34edb83 > examples/cpp/connection_options.cpp PRE-CREATION > proton-c/bindings/cpp/CMakeLists.txt 9f423fb > proton-c/bindings/cpp/include/proton/connection_options.hpp PRE-CREATION > proton-c/bindings/cpp/include/proton/container.hpp 059416f > proton-c/bindings/cpp/include/proton/reconnect_timer.hpp PRE-CREATION > proton-c/bindings/cpp/include/proton/transport.hpp 3d602b7 > proton-c/bindings/cpp/src/blocking_connection_impl.cpp 8abd24b > proton-c/bindings/cpp/src/connection.cpp 1b35e03 > proton-c/bindings/cpp/src/connection_options.cpp PRE-CREATION > proton-c/bindings/cpp/src/connector.hpp b311cc9 > proton-c/bindings/cpp/src/connector.cpp 49660ea > proton-c/bindings/cpp/src/container.cpp 13f12d9 > proton-c/bindings/cpp/src/container_impl.hpp ec356e2 > proton-c/bindings/cpp/src/container_impl.cpp fc97a3e > proton-c/bindings/cpp/src/reconnect_timer.cpp PRE-CREATION > proton-c/bindings/cpp/src/transport.cpp 58114ae > > Diff: https://reviews.apache.org/r/39694/diff/ > > > Testing > ------- > > Linux only > > > Thanks, > > Cliff Jansen > >
