You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@metron.apache.org by nickwallen <gi...@git.apache.org> on 2017/09/08 15:30:04 UTC

[GitHub] metron pull request #740: METRON-1167 Define Session Specific Global Configu...

GitHub user nickwallen opened a pull request:

    https://github.com/apache/metron/pull/740

    METRON-1167 Define Session Specific Global Configuration Values in the REPL

    Many Stellar functions accept configuration values from the Global configuration.  When using the REPL you can load the global configuration values by launching the REPL with the -z option, which loads the global configuration stored in Zookeeper.
    
    The only way to modify the global configuration within the REPL currently is to do the following steps.
    1. Retrieve the global configuration with conf := CONFIG_GET("global")
    2. Alter that global configuration by modifying the JSON contained in the `conf` variable.
    3. Push the new global configuration using CONFIG_PUT("global", conf)
    4. Close and then reopen the REPL.  Without restarting the REPL the new global configuration is not loaded.
    
    I want a way to do this directly in the REPL, without restarting it, and also in a way that does not modify the persisted global configuration in Zookeeper.  I may be monkeying with something the REPL, but I don't want to break one of my live Metron topologies.
    
    ### Test Drive
    
    1. Check-out this PR and build Metron. 
         ```
         mvn clean install -DskipTests -T1C
         ```
    
    1. Then from the root of the Metron source directory, launch the REPL.
        ```
        $ mvn exec:java \
            -Dexec.mainClass="org.apache.metron.stellar.common.shell.StellarShell" \
            -pl metron-stellar/stellar-common/
        ```
    
    1. Then ``%define`, `%undefine`, and `%globals` to your heart's content.
        ```
        [Stellar]>>> %globals
        {}
        [Stellar]>>> %define foo = bar
        [Stellar]>>> %globals
        {foo=bar}
        [Stellar]>>> %define baz=bash
        [Stellar]>>> %globals
        {foo=bar, baz=bash}
        [Stellar]>>> %undefine foo
        [Stellar]>>> %globals
        {baz=bash}
        ```

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/nickwallen/metron METRON-1167

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/metron/pull/740.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #740
    
----
commit dd91135a551a493ae9823db673c485824f2fe20c
Author: Nick Allen <ni...@nickallen.org>
Date:   2017-09-07T22:58:58Z

    METRON-1167 Define Session Specific Global Configuration Values in the REPL

----


---

[GitHub] metron pull request #740: METRON-1167 Define Session Specific Global Configu...

Posted by ottobackwards <gi...@git.apache.org>.

Github user ottobackwards commented on a diff in the pull request:

    https://github.com/apache/metron/pull/740#discussion_r137841201
  
    --- Diff: metron-stellar/stellar-common/src/main/java/org/apache/metron/stellar/common/shell/StellarShell.java ---
    @@ -308,17 +315,68 @@ private void handleMagic( String rawExpression) {
                   .collect(Collectors.joining(", "));
           writeLine(functions);
     
    -    } else if(MAGIC_VARS.equals(command)) {
    +    } else if (MAGIC_VARS.equals(command)) {
     
    -      // list all variables
    +      // '%vars' -> list all variables in scope
           executor.getVariables()
    -              .forEach((k,v) -> writeLine(String.format("%s = %s", k, v)));
    +              .forEach((k, v) -> writeLine(String.format("%s = %s", k, v)));
    +
    +    } else if (MAGIC_GLOBALS.equals(command)) {
    +
    +      // '%globals' -> list all globals in scope
    +      Map<String, Object> globals = Collections.emptyMap();
    --- End diff --
    
    Never mind, calling getOrCreate will put the capability in. I see. Sorry


---

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

Posted by ottobackwards <gi...@git.apache.org>.

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<string,object> all the time?


---

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

Posted by cestella <gi...@git.apache.org>.

Github user cestella commented on the issue:

    https://github.com/apache/metron/pull/740
  
    yeah, we can definitely have complex types in the global config; that gets serialized as a `Map<String, Object>` and jackson will happily load a map of maps or lists.  The idea originally was that the global config could, for instance, hold static reference data that was super small, which could include maps.


---

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

Posted by ottobackwards <gi...@git.apache.org>.

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 ...

Posted by nickwallen <gi...@git.apache.org>.

Github user nickwallen commented on the issue:

    https://github.com/apache/metron/pull/740
  
    What flag am I co-opting?


---

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

Posted by ottobackwards <gi...@git.apache.org>.

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 partial configuration?
    



---

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

Posted by nickwallen <gi...@git.apache.org>.

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<string,object> all the time?
    
    It still is always a map<string, object>, that has not changed. When I said "How about we just restrict %define to strings only for now.", that just means that a `%define` drops into the map<string, object>, values of strings without trying to interpret the value and create a complex type.
    
    The main itch that I'm trying to scratch here (which is maybe the 80% case?) is to just set a simple property value.
    ```
    %define bootstrap.servers=node1:6667
    KAFKA_GET("indexing", 10)
    ```
    
    If we treat `%define` arguments as JSON (so it behaves just like the `-z`) then it would have to look something like this, which is way more quotes and parens than I'd like to deal with.
    ```
    %define { "bootstrap.servers" : "node1:6667" }
    KAFKA_GET("indexing", 10)
    ```


---

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

Posted by nickwallen <gi...@git.apache.org>.

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 "CONFIG_GET, CONFIG_PUT, restart REPL" dance that I described in the PR description.


---