[GitHub] metron issue #856: METRON-1339 Stellar Shell functionality to verify stored ...
Github user ottobackwards commented on the issue: https://github.com/apache/metron/pull/856 Refactored based on feedback for some things, based on making what I was trying for more correct in others. ---
[GitHub] metron issue #856: METRON-1339 Stellar Shell functionality to verify stored ...
Github user ottobackwards commented on the issue: https://github.com/apache/metron/pull/856 Ok, I'll change it. Feels a little crossing the streams, but we'll see ---
[GitHub] metron issue #856: METRON-1339 Stellar Shell functionality to verify stored ...
Github user nickwallen commented on the issue: https://github.com/apache/metron/pull/856 The justification that you mentioned just doesn't seem strong enough to me. Unless there is more that I am missing. IMHO We should only use magic commands for things that can't be accomplished in the language using the preferred extension mechanism; aka defining Stellar functions. ---
[GitHub] metron issue #856: METRON-1339 Stellar Shell functionality to verify stored ...
Github user ottobackwards commented on the issue: https://github.com/apache/metron/pull/856 Do you feel strongly that this should be a Function? @cestella ? I'm not opposed to changing it if you are. I would like to here some more feedback ---
[GitHub] metron issue #856: METRON-1339 Stellar Shell functionality to verify stored ...
Github user ottobackwards commented on the issue: https://github.com/apache/metron/pull/856 It did not seem appropriate to me for this to be a stellar function, and %magic is the other way to execute things from the shell. At the time at least. ---
[GitHub] metron issue #856: METRON-1339 Stellar Shell functionality to verify stored ...
Github user nickwallen commented on the issue: https://github.com/apache/metron/pull/856 Why is this validation process driven by a %magic command? Magics were made for functionality that cannot be implemented directly within a Stellar execution environment. Often for answering questions 'about' the execution environment itself. For example `%vars` or `%functions` or `%globals` all tell us about the Stellar execution environment. This logic doesn't seem like a good fit for a %magic, unless there is a limitation that I am not understanding. This could be implemented, I would argue more simply, as a regular Stellar function. ---
[GitHub] metron issue #856: METRON-1339 Stellar Shell functionality to verify stored ...
Github user ottobackwards commented on the issue: https://github.com/apache/metron/pull/856 @nickwallen yeah, we need to cover a bunch a scenarios ---
[GitHub] metron issue #856: METRON-1339 Stellar Shell functionality to verify stored ...
Github user nickwallen commented on the issue: https://github.com/apache/metron/pull/856 @ottobackwards Yes, definitely. I like it at a 50k foot level. The only thing that struck me was the need for the different annotation types. But I haven't had a chance to dig into it yet. ---
[GitHub] metron issue #856: METRON-1339 Stellar Shell functionality to verify stored ...
Github user ottobackwards commented on the issue: https://github.com/apache/metron/pull/856 @nickwallen any feedback, does the annotated approach match what you imagined? ---
[GitHub] metron issue #856: METRON-1339 Stellar Shell functionality to verify stored ...
Github user ottobackwards commented on the issue: https://github.com/apache/metron/pull/856 Closing to test build in travis ---
[GitHub] metron issue #856: METRON-1339 Stellar Shell functionality to verify stored ...
Github user ottobackwards commented on the issue: https://github.com/apache/metron/pull/856 Bump? ---
[GitHub] metron issue #856: METRON-1339 Stellar Shell functionality to verify stored ...
Github user ottobackwards commented on the issue: https://github.com/apache/metron/pull/856 @cestella @simonellistonball With the new implementation, doing the blob or file check should be a piece of cake would you prefer it as part of this or as a new issue? ---
[GitHub] metron issue #856: METRON-1339 Stellar Shell functionality to verify stored ...
Github user ottobackwards commented on the issue: https://github.com/apache/metron/pull/856  ---
[GitHub] metron issue #856: METRON-1339 Stellar Shell functionality to verify stored ...
Github user ottobackwards commented on the issue: https://github.com/apache/metron/pull/856  ---
[GitHub] metron issue #856: METRON-1339 Stellar Shell functionality to verify stored ...
Github user ottobackwards commented on the issue: https://github.com/apache/metron/pull/856  ---
[GitHub] metron issue #856: METRON-1339 Stellar Shell functionality to verify stored ...
Github user ottobackwards commented on the issue: https://github.com/apache/metron/pull/856 https://github.com/apache/metron/pull/856/commits/65278a67a07f1c4c23ab2d95ebb6de92e1cac731 introduces conceptually what @nickwallen and I have been discussing. I need to think about reworking the description. From the commit -> Refactor based on review and inspiration from review. Although the original implementation was functional, it required maintainence to keep current. The suggested 'best state' was to have it be possible, maybe through annotations, for the validation system to be able to handle any config, regarless or composition using annotations. That would leave it up to the implementor to propertly annotate thier configurations, and allow for support of new fields. This is an implementation of that. I have refactored the implemenations and details, but kept the discovery and mechanics ( loading and visitation ) somewhat the same. Hopefully keeping the good and reworking to a more sustainable solution. Several annotations where created to marks ceratin stellar configruation objects or scenarios. A holder object, to hold the configuration object, but knows how to process the annotations and run the visitation was added. This holder object and the annotations have parameters and handling for several special scenarios, such as 2x nested maps. This implementation should facilitate follow on work to validate files and streams and blobs by using implementing the StellarValidator interface and re-using the holder concept ( replacing the providers ) ---
[GitHub] metron issue #856: METRON-1339 Stellar Shell functionality to verify stored ...
Github user ottobackwards commented on the issue: https://github.com/apache/metron/pull/856 Per the conversation above, i'm going to take a stab at the attributed approach. I think the Stellar Functions should be a separate Jira. ---
[GitHub] metron issue #856: METRON-1339 Stellar Shell functionality to verify stored ...
Github user ottobackwards commented on the issue: https://github.com/apache/metron/pull/856 I am glad for the interest in this PR, and that it seems to have sparked some great ideas for continuing on. What I would like to do is line it up as follows 1. This PR with it's current scope and focus 2. New Jira and PR(s) for the Stellar Functions/Namespace that @cestella and @simonellistonball mentioned 3. Some research and possible prototyping of the Attributed approach @nickwallen has suggested ( which I agree with ) Over the course of that work, and other work identified through working and reviewing it, I will iteratively refactor to a common code and reusable approach. ---
[GitHub] metron issue #856: METRON-1339 Stellar Shell functionality to verify stored ...
Github user cestella commented on the issue: https://github.com/apache/metron/pull/856 @simonellistonball Agree to the namespace idea. My bad :) ---
[GitHub] metron issue #856: METRON-1339 Stellar Shell functionality to verify stored ...
Github user ottobackwards commented on the issue: https://github.com/apache/metron/pull/856 @simonellistonball, yes, the namespace should be part of the jira and interface design ---
[GitHub] metron issue #856: METRON-1339 Stellar Shell functionality to verify stored ...
Github user ottobackwards commented on the issue: https://github.com/apache/metron/pull/856 So, the scenario here is checking things that *were* valid when uploaded, but have been invalidated by external changes ( language changes ). I would like to keep the magic specific. I think the functionality for the management functions is valid, but can we do that as a separate Jira/PR? I'll do it, I just want to keep this tight. If you create the jira and assign it to me that would be super. I would do the files on disk using the management functions as well. So we just need to think of the stellar interface for calling `VALIDATE` with a string, and with a file path. Also saying what configuration type it is. Does that make sense? ---
[GitHub] metron issue #856: METRON-1339 Stellar Shell functionality to verify stored ...
Github user simonellistonball commented on the issue: https://github.com/apache/metron/pull/856 @cestella I would say that proposed validate function has to be very much in a namespace. It feels like a name that would be much more useful for a function replacing our current approach to global validation in the future than config validation, other than that it sounds like a good idea. ---
[GitHub] metron issue #856: METRON-1339 Stellar Shell functionality to verify stored ...
Github user cestella commented on the issue: https://github.com/apache/metron/pull/856 Also, it might be useful for `%validate_configured_expressions` to take a file path so you can validate a set of configs on disk (again, if it gets to zookeeper, zk_load_utils.sh should fail if it's invalid) ---
[GitHub] metron issue #856: METRON-1339 Stellar Shell functionality to verify stored ...
Github user cestella commented on the issue: https://github.com/apache/metron/pull/856 Any chance we can add a `VALIDATE(str, type)` function to the stellar management functions where str is the json blob string for the config and the type is the type of config? Generally the goal is to disallow invalid stellar to get pushed to zookeeper via `zk_load_utils.sh`, so I suspect a function would be more useful in the situation where you're constructing a config in the REPL via management functions and want to validate it before pushing it. ---
