[GitHub] nifi issue #501: NIFI-1974 - Support Custom Properties in Expression Languag...
Github user markap14 commented on the issue: https://github.com/apache/nifi/pull/501 Yolanda - I have merged to 0.x. This was very well done. I like the class hierarchy you put together - the breakout of interfaces, abstract classes, and concrete classes. I think it will yield itself nicely to the future updates so that this can be baked into the UI. Nicely done! Thanks for jumping in and knocking this out! --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] nifi issue #501: NIFI-1974 - Support Custom Properties in Expression Languag...
Github user markap14 commented on the issue: https://github.com/apache/nifi/pull/501 @YolandaMDavis All looks good with the exception of needed to trim() the filenames and one other thing that i noticed, which is that if multiple properties files are specified and one cannot be loaded, none are loaded. I will create an additional commit just to log this accurately and trim the filename and push this to 0.x. I created a JIRA https://issues.apache.org/jira/browse/NIFI-2057 to address the multiple files better, but I don't want that to hold up the 0.7.0 release or prevent this from making it into 0.7.0. I will merge this as-is with that minor change noted to 0.x. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] nifi issue #501: NIFI-1974 - Support Custom Properties in Expression Languag...
Github user YolandaMDavis commented on the issue: https://github.com/apache/nifi/pull/501 Per my discussion with @markap14 having ControllerServiceLookup extend VariableRegistry interface did not fit in with the overall purpose of the ControllerServiceLookup interface. Also in some cases service lookups are not required objects, so passing the variable registry directly is optimal. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] nifi issue #501: NIFI-1974 - Support Custom Properties in Expression Languag...
Github user joewitt commented on the issue: https://github.com/apache/nifi/pull/501 @YolandaMDavis can you please be sure to list the key points of the offline discussion here? Good to capture the outcome but even better to capture the thinking for the extended community to participate --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] nifi issue #501: NIFI-1974 - Support Custom Properties in Expression Languag...
Github user YolandaMDavis commented on the issue: https://github.com/apache/nifi/pull/501 @markap14 per our offline discussion I made changes that provides variable registry via constructors and eliminated the access from the ControllerServiceLookup. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] nifi issue #501: NIFI-1974 - Support Custom Properties in Expression Languag...
Github user YolandaMDavis commented on the issue: https://github.com/apache/nifi/pull/501 @markap14 Thanks for reviewing! Concerning the extension of VariableRegistryProvider by the ControllerServiceLookup, it came from the need to populate the VariableRegistry from NiFiProperties, if available, (which was provided by implementations of ControllerServiceLookup, including FlowController, WebClusterManager) and provide it to StatePropertyValue (which received a ControllerServiceLookup object in it's constructor). I thought about having a third variable in the constructor for StatePropertyValue for the variableRegistry however when attempting to implement I needed to interrogate the ControllerServiceLookup anyway in many cases. I definitely understand the weirdness which is why a the least I had it extend the interface as opposed to adding the getVariableRegistry method to that interface directly. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] nifi issue #501: NIFI-1974 - Support Custom Properties in Expression Languag...
Github user markap14 commented on the issue: https://github.com/apache/nifi/pull/501 @YolandaMDavis I left several comments inline. Only other comment that I have is that it feels a little weird to me to have `ControllerServiceLookup` extend `VariableRegistryProvider`. These are really unrelated concepts... any insight as to why you went that route? Otherwise, all is looking great! Certainly not a trivial addition to the codebase. Thanks for sticking with it to get all of this knocked out! --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
