[GitHub] metron issue #740: METRON-1167 Define Session Specific Global Configuration ...

Github user nickwallen commented on the issue: https://github.com/apache/metron/pull/740 I see this feedback as related to the design/use of global properties in general, versus anything that this PR does. I'd suggest opening a discuss thread on this IMHO. Nothing wrong

[GitHub] metron issue #740: METRON-1167 Define Session Specific Global Configuration ...

Github user ottobackwards commented on the issue: https://github.com/apache/metron/pull/740 I might be thinking a little too far down the road, sorry ---

[GitHub] metron issue #740: METRON-1167 Define Session Specific Global Configuration ...

Github user ottobackwards commented on the issue: https://github.com/apache/metron/pull/740 - So we need to document what the flag actually means - We need UDF writers to understand the flag and how to guard for missing properties - Need the users to understand that the

[GitHub] metron issue #740: METRON-1167 Define Session Specific Global Configuration ...

Github user ottobackwards commented on the issue: https://github.com/apache/metron/pull/740 "There really is no such thing as an "incompletely formed" GLOBAL_CONFIG. The GLOBAL_CONFIG is just a key/value store. It contains whatever it contains. But if there is a missing value in the

[GitHub] metron issue #740: METRON-1167 Define Session Specific Global Configuration ...

Github user nickwallen commented on the issue: https://github.com/apache/metron/pull/740 This is a replay of an offline conversation > Sorry for the confusion on my part @ottobackwards . I know much of the innards of Stellar is only documented in the code (if that), so

[GitHub] metron issue #740: METRON-1167 Define Session Specific Global Configuration ...

Github user ottobackwards commented on the issue: https://github.com/apache/metron/pull/740 I am +1 on this. While I think it is confusing to conceptually have a GLOBAL_CONF wrt the zookeeper global.js, good documentation and community feedback from use will get us there.

[GitHub] metron issue #740: METRON-1167 Define Session Specific Global Configuration ...

Github user ottobackwards commented on the issue: https://github.com/apache/metron/pull/740 So the GLOBAL_CONFIG is used when setting up stellar in a few places, and used in a couple of functions to determine what they return. And may be used by other functions in the future both

[GitHub] metron issue #740: METRON-1167 Define Session Specific Global Configuration ...

Github user nickwallen commented on the issue: https://github.com/apache/metron/pull/740 > What about functions that check for the GLOBAL_ flag, and may make assumptions that do not hold with a non-z partial configuration? I thought I had responded to this, but I must not

[GitHub] metron issue #740: METRON-1167 Define Session Specific Global Configuration ...

Github user ottobackwards commented on the issue: https://github.com/apache/metron/pull/740 thanks @cestella. I am waiting on an answer to the question on other functions that look for the GLOBAL_CONFIG flag. See above, a couple of times. ---

[GitHub] metron issue #740: METRON-1167 Define Session Specific Global Configuration ...

Github user cestella commented on the issue: https://github.com/apache/metron/pull/740 YES! I love it. We can create a follow-on for a `-g` option to initialize the global config, but I like this approach. +1 by inspection, but I want to make sure @ottobackwards is cool with it as

[GitHub] metron issue #740: METRON-1167 Define Session Specific Global Configuration ...

Github user nickwallen commented on the issue: https://github.com/apache/metron/pull/740 I am re-reading through everyone's comments just to make sure I haven't misjudged all the feedback that I have received so far (thanks to all for feedback). > you could also make

[GitHub] metron issue #740: METRON-1167 Define Session Specific Global Configuration ...

Github user nickwallen commented on the issue: https://github.com/apache/metron/pull/740 > Can't we turn everything passed in into json and use the json patch stuff to "build out' the configuration? Why not have the globals as a map all the time? It still is

[GitHub] metron issue #740: METRON-1167 Define Session Specific Global Configuration ...

Github user ottobackwards commented on the issue: https://github.com/apache/metron/pull/740 Even if they don't now, I believe that is the intent of the feature ---

[GitHub] metron issue #740: METRON-1167 Define Session Specific Global Configuration ...

Github user ottobackwards commented on the issue: https://github.com/apache/metron/pull/740 I don't know if you saw my question, and it is hidden now so I'll restate: What about functions that check for the GLOBAL_ flag, and may make assumptions that do not hold with a non-z

[GitHub] metron issue #740: METRON-1167 Define Session Specific Global Configuration ...

Github user ottobackwards commented on the issue: https://github.com/apache/metron/pull/740 Can't we turn everything passed in into json and use the json patch stuff to "build out' the configuration? Why not have the globals as a map all the time? ---

[GitHub] metron issue #740: METRON-1167 Define Session Specific Global Configuration ...

Github user nickwallen commented on the issue: https://github.com/apache/metron/pull/740 "a bit more complexity than I'd like" may just need to be read as "Nick is lazy", but I'll let you be the judge ---

[GitHub] metron issue #740: METRON-1167 Define Session Specific Global Configuration ...

Github user nickwallen commented on the issue: https://github.com/apache/metron/pull/740 Those options seem to add a bit more complexity than I'd like. How about we just restrict %define to strings only for now. If you need to use complex types, then you can still do the

[GitHub] metron issue #740: METRON-1167 Define Session Specific Global Configuration ...

Github user cestella commented on the issue: https://github.com/apache/metron/pull/740 you could also make `%define nicks_new_global_field := my_complex_field` That makes somewhat more sense since stellar assignment is `:=` and property assignment is `=` ---

[GitHub] metron issue #740: METRON-1167 Define Session Specific Global Configuration ...

Github user cestella commented on the issue: https://github.com/apache/metron/pull/740 So, one way to do this is to allow users to refer to stellar fields: ``` my_complex_field := { 'blah' : 7 } %define nicks_new_global_field=_complex_field ``` Another option

[GitHub] metron issue #740: METRON-1167 Define Session Specific Global Configuration ...

Github user nickwallen commented on the issue: https://github.com/apache/metron/pull/740 And in terms of the complex types I can see the use case definitely. In terms of the REPL and the changes in this PR, a `%define` does the same as when GLOBAL_CONFIG gets created with a `-z` for

[GitHub] metron issue #740: METRON-1167 Define Session Specific Global Configuration ...

Github user cestella commented on the issue: https://github.com/apache/metron/pull/740 so, what happens if we modify a field via `%define` from a stellar shell started with `-z` ? ---

[GitHub] metron issue #740: METRON-1167 Define Session Specific Global Configuration ...

Github user nickwallen commented on the issue: https://github.com/apache/metron/pull/740 Yes, I think it would be useful to pass in a global config as a command line argument. Maybe something like... ``` bin/stellar -g config/zookeeper/global.json ``` I think we

[GitHub] metron issue #740: METRON-1167 Define Session Specific Global Configuration ...

Github user cestella commented on the issue: https://github.com/apache/metron/pull/740 I think I'd be ok without that capability if we had an ability to pass in a global config as a JSON blob, like we do the initial set of variables. What do you think? ---

[GitHub] metron issue #740: METRON-1167 Define Session Specific Global Configuration ...

Github user cestella commented on the issue: https://github.com/apache/metron/pull/740 How would we, say, specify a global config value as a map (e.g. foo={'bar' : 'blah'})? global config is also exposed as variables stellar in the parser and enrichments, so this could very well be