[Bro-Dev] [JIRA] (BIT-1319) topic/jsiwek/broker

[Robin Sommer (JIRA)]

    [ https://bro-tracker.atlassian.net/browse/BIT-1319?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=19902#comment-19902 ] 

Robin Sommer commented on BIT-1319:
-----------------------------------

Merged. Looked primarily at the Bro parts; it's nice to see this all fitting in without too much surgery, seems the interface is right.

I have a few smaller points and some broader thoughts/questions:

- cmake/RequireCXX11.cmake says: {{TODO: don't seem to be any great/easy ways to get a clang version string.}} Isn't that as easy as: {{clang --version | grep ^clang | cut -d ' ' -f 3}} ?

- in the Bro docs for the Broker interface, I think it would be helpful to revert the order of the consumer/producer examples to show producer/consumer instead. In particular for the Store example, it took me a bit to realize some missing context is really in the 2nd script.

- {{Store::create_clone(“name”)}}: I'm not quite sure how to interpret this in terms of which peer this goes out to: is it cloning any store of that name, independent of the peer? What if two peers both happen to have a store with that name? Should the function explicitly specify the peer instead?

- two tests don't terminate for me (the 2nd one I have to kill, presumably because it doesn't use btest-bg-wait)

{code}
[  0%] comm.clone_store ... failed
  % 'btest-bg-wait 20' failed unexpectedly (exit code 1)
  % cat .stderr
  The following processes did not terminate:

  bro -b ../clone.bro broker_port=9999/tcp >clone.out
  bro -b ../master.bro broker_port=9999/tcp >master.out

  -----------
  <<< [906] bro -b ../clone.bro broker_port=9999/tcp >clone.out
  <params>, line 1: received termination signal
  >>>
  <<< [985] bro -b ../master.bro broker_port=9999/tcp >master.out
  <params>, line 1: received termination signal
  >>>

[ 33%] comm.master_store ...^C
{code}

- I was wondering about namespaces for the broker parts, both script-land and C++. I'm kind of inclined to just call it {{Broker}}, and maybe {{BrokerComm}} and {{BrokerStore}} in script-land. That way it's clear what it's about. The script framework would then also become {{broker}}.

- The script API for the Store looks a bit cumbersome to use, because of the async interface through when. Could we add sync versions of the various functions that just go to the local cache? Or does that not work architecturally with how the communication between Bro/Broker/CAF is structured?

- I also wondered about this: {{Comm::refine_to_string(Comm::vector_lookup(res$result, 0)));}} That also looks somewhat cumbersome, but I haven't further traced down yet what exactly is happening there.


> topic/jsiwek/broker
> -------------------
>
>                 Key: BIT-1319
>                 URL: https://bro-tracker.atlassian.net/browse/BIT-1319
>             Project: Bro Issue Tracker
>          Issue Type: New Feature
>          Components: Bro
>            Reporter: Jon Siwek
>            Assignee: Robin Sommer
>             Fix For: 2.4
>
>
> The "topic/jsiwek/broker" branch is in the bro and cmake repos to add the initial support for Broker.
> Notes/Disclaimers/Caveats:
> - Bro has a --enable-broker configure flag.
> - requires actor-framework "develop" branch.  When version 0.13 is out, I will put that as a requirement in the README and have CMake check for that.
> - no C bindings yet
> - no Python bindings yet
> - other than checking compilation that the new unit tests pass on Linux/FreeBSD/Mac, I've not done must extensive of testing, profiling, optimization etc.



--
This message was sent by Atlassian JIRA
(v6.4-OD-15-055#64014)