[jira] [Commented] (NIFI-5073) JMSConnectionFactory doesn't resolve 'variables' properly
[ https://issues.apache.org/jira/browse/NIFI-5073?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16489286#comment-16489286 ] Sivaprasanna Sethuraman commented on NIFI-5073: --- Great. I'm changing this to 'Resolved' > JMSConnectionFactory doesn't resolve 'variables' properly > - > > Key: NIFI-5073 > URL: https://issues.apache.org/jira/browse/NIFI-5073 > Project: Apache NiFi > Issue Type: Bug > Components: Core Framework >Affects Versions: 1.5.0, 1.6.0 >Reporter: Matthew Clarke >Assignee: Sivaprasanna Sethuraman >Priority: Major > Fix For: 1.7.0 > > Attachments: > 0001-NIFI-5073-JMSConnectionFactoryProvider-now-resolves-.patch > > > Create a new process Group. > Add "Variables" to the process group: > for example: > broker_uri=tcp://localhost:4141 > client_libs=/NiFi/custom-lib-dir/MQlib > con_factory=blah > Then while that process group is selected, create a controller service. > Create JMSConnectionFactory. > Configure this controller service to use EL for PG defined variables above: > ${con_factory}, ${con_factory}, and ${broker_uri} > The controller service will remain invalid because the EL statements are not > properly resolved to their set values. > Doing the exact same thing above using the external NiFi registry file works > as expected. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (NIFI-5073) JMSConnectionFactory doesn't resolve 'variables' properly
[ https://issues.apache.org/jira/browse/NIFI-5073?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16489224#comment-16489224 ] ASF GitHub Bot commented on NIFI-5073: -- Github user markap14 commented on the issue: https://github.com/apache/nifi/pull/2653 @zenfenan thanks for the update! The only thing I think that's missing is that you left in the code to change the ClassLoader, and that can be removed now because the framework is now managing the classloader. I removed it locally, tested, and all looks good. Good looks good. +1 merged to master. Thanks for the fix - and sorry it took so long to get back to it! > JMSConnectionFactory doesn't resolve 'variables' properly > - > > Key: NIFI-5073 > URL: https://issues.apache.org/jira/browse/NIFI-5073 > Project: Apache NiFi > Issue Type: Bug > Components: Core Framework >Affects Versions: 1.5.0, 1.6.0 >Reporter: Matthew Clarke >Assignee: Sivaprasanna Sethuraman >Priority: Major > Attachments: > 0001-NIFI-5073-JMSConnectionFactoryProvider-now-resolves-.patch > > > Create a new process Group. > Add "Variables" to the process group: > for example: > broker_uri=tcp://localhost:4141 > client_libs=/NiFi/custom-lib-dir/MQlib > con_factory=blah > Then while that process group is selected, create a controller service. > Create JMSConnectionFactory. > Configure this controller service to use EL for PG defined variables above: > ${con_factory}, ${con_factory}, and ${broker_uri} > The controller service will remain invalid because the EL statements are not > properly resolved to their set values. > Doing the exact same thing above using the external NiFi registry file works > as expected. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (NIFI-5073) JMSConnectionFactory doesn't resolve 'variables' properly
[ https://issues.apache.org/jira/browse/NIFI-5073?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16489223#comment-16489223 ] ASF GitHub Bot commented on NIFI-5073: -- Github user asfgit closed the pull request at: https://github.com/apache/nifi/pull/2653 > JMSConnectionFactory doesn't resolve 'variables' properly > - > > Key: NIFI-5073 > URL: https://issues.apache.org/jira/browse/NIFI-5073 > Project: Apache NiFi > Issue Type: Bug > Components: Core Framework >Affects Versions: 1.5.0, 1.6.0 >Reporter: Matthew Clarke >Assignee: Sivaprasanna Sethuraman >Priority: Major > Attachments: > 0001-NIFI-5073-JMSConnectionFactoryProvider-now-resolves-.patch > > > Create a new process Group. > Add "Variables" to the process group: > for example: > broker_uri=tcp://localhost:4141 > client_libs=/NiFi/custom-lib-dir/MQlib > con_factory=blah > Then while that process group is selected, create a controller service. > Create JMSConnectionFactory. > Configure this controller service to use EL for PG defined variables above: > ${con_factory}, ${con_factory}, and ${broker_uri} > The controller service will remain invalid because the EL statements are not > properly resolved to their set values. > Doing the exact same thing above using the external NiFi registry file works > as expected. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (NIFI-5073) JMSConnectionFactory doesn't resolve 'variables' properly
[ https://issues.apache.org/jira/browse/NIFI-5073?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16489219#comment-16489219 ] ASF subversion and git services commented on NIFI-5073: --- Commit db259628c7ea359c0783a3cb800f5d58da9e09ed in nifi's branch refs/heads/master from [~sivaprasanna] [ https://git-wip-us.apache.org/repos/asf?p=nifi.git;h=db25962 ] NIFI-5073: JMSConnectionFactoryProvider now resolves EL Expression from VariableRegistry - CLIENT_LIB_PATH is updated to include 'dynamicallyModifiesClasspath(true)' > JMSConnectionFactory doesn't resolve 'variables' properly > - > > Key: NIFI-5073 > URL: https://issues.apache.org/jira/browse/NIFI-5073 > Project: Apache NiFi > Issue Type: Bug > Components: Core Framework >Affects Versions: 1.5.0, 1.6.0 >Reporter: Matthew Clarke >Assignee: Sivaprasanna Sethuraman >Priority: Major > Attachments: > 0001-NIFI-5073-JMSConnectionFactoryProvider-now-resolves-.patch > > > Create a new process Group. > Add "Variables" to the process group: > for example: > broker_uri=tcp://localhost:4141 > client_libs=/NiFi/custom-lib-dir/MQlib > con_factory=blah > Then while that process group is selected, create a controller service. > Create JMSConnectionFactory. > Configure this controller service to use EL for PG defined variables above: > ${con_factory}, ${con_factory}, and ${broker_uri} > The controller service will remain invalid because the EL statements are not > properly resolved to their set values. > Doing the exact same thing above using the external NiFi registry file works > as expected. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (NIFI-5073) JMSConnectionFactory doesn't resolve 'variables' properly
[ https://issues.apache.org/jira/browse/NIFI-5073?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16489220#comment-16489220 ] ASF subversion and git services commented on NIFI-5073: --- Commit 1597492fed749c13a97f44d0d8ed633787702d3c in nifi's branch refs/heads/master from [~markap14] [ https://git-wip-us.apache.org/repos/asf?p=nifi.git;h=1597492 ] NIFI-5073: Removed unneeded code for changing ClassLoader since it is now managed by framework This closes #2653. > JMSConnectionFactory doesn't resolve 'variables' properly > - > > Key: NIFI-5073 > URL: https://issues.apache.org/jira/browse/NIFI-5073 > Project: Apache NiFi > Issue Type: Bug > Components: Core Framework >Affects Versions: 1.5.0, 1.6.0 >Reporter: Matthew Clarke >Assignee: Sivaprasanna Sethuraman >Priority: Major > Attachments: > 0001-NIFI-5073-JMSConnectionFactoryProvider-now-resolves-.patch > > > Create a new process Group. > Add "Variables" to the process group: > for example: > broker_uri=tcp://localhost:4141 > client_libs=/NiFi/custom-lib-dir/MQlib > con_factory=blah > Then while that process group is selected, create a controller service. > Create JMSConnectionFactory. > Configure this controller service to use EL for PG defined variables above: > ${con_factory}, ${con_factory}, and ${broker_uri} > The controller service will remain invalid because the EL statements are not > properly resolved to their set values. > Doing the exact same thing above using the external NiFi registry file works > as expected. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (NIFI-5073) JMSConnectionFactory doesn't resolve 'variables' properly
[ https://issues.apache.org/jira/browse/NIFI-5073?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16487485#comment-16487485 ] ASF GitHub Bot commented on NIFI-5073: -- Github user zenfenan commented on a diff in the pull request: https://github.com/apache/nifi/pull/2653#discussion_r190296123 --- Diff: nifi-nar-bundles/nifi-jms-bundle/nifi-jms-processors/src/main/java/org/apache/nifi/jms/cf/JMSConnectionFactoryProvider.java --- @@ -159,13 +159,15 @@ public void enable(ConfigurationContext context) throws InitializationException if (logger.isInfoEnabled()) { logger.info("Configuring " + this.getClass().getSimpleName() + " for '" + context.getProperty(CONNECTION_FACTORY_IMPL).evaluateAttributeExpressions().getValue() + "' to be connected to '" -+ BROKER_URI + "'"); ++ context.getProperty(BROKER_URI).evaluateAttributeExpressions().getValue() + "'"); } + // will load user provided libraries/resources on the classpath - Utils.addResourcesToClasspath(context.getProperty(CLIENT_LIB_DIR_PATH).evaluateAttributeExpressions().getValue()); +final String clientLibPath = context.getProperty(CLIENT_LIB_DIR_PATH).evaluateAttributeExpressions().getValue(); +ClassLoader customClassLoader = ClassLoaderUtils.getCustomClassLoader(clientLibPath, this.getClass().getClassLoader(), null); + Thread.currentThread().setContextClassLoader(customClassLoader); --- End diff -- @markap14 Appreciate if you could take a look at this. > JMSConnectionFactory doesn't resolve 'variables' properly > - > > Key: NIFI-5073 > URL: https://issues.apache.org/jira/browse/NIFI-5073 > Project: Apache NiFi > Issue Type: Bug > Components: Core Framework >Affects Versions: 1.5.0, 1.6.0 >Reporter: Matthew Clarke >Assignee: Sivaprasanna Sethuraman >Priority: Major > Attachments: > 0001-NIFI-5073-JMSConnectionFactoryProvider-now-resolves-.patch > > > Create a new process Group. > Add "Variables" to the process group: > for example: > broker_uri=tcp://localhost:4141 > client_libs=/NiFi/custom-lib-dir/MQlib > con_factory=blah > Then while that process group is selected, create a controller service. > Create JMSConnectionFactory. > Configure this controller service to use EL for PG defined variables above: > ${con_factory}, ${con_factory}, and ${broker_uri} > The controller service will remain invalid because the EL statements are not > properly resolved to their set values. > Doing the exact same thing above using the external NiFi registry file works > as expected. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (NIFI-5073) JMSConnectionFactory doesn't resolve 'variables' properly
[ https://issues.apache.org/jira/browse/NIFI-5073?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16473395#comment-16473395 ] ASF GitHub Bot commented on NIFI-5073: -- Github user zenfenan commented on a diff in the pull request: https://github.com/apache/nifi/pull/2653#discussion_r187790872 --- Diff: nifi-nar-bundles/nifi-jms-bundle/nifi-jms-processors/src/main/java/org/apache/nifi/jms/cf/JMSConnectionFactoryProvider.java --- @@ -159,13 +159,15 @@ public void enable(ConfigurationContext context) throws InitializationException if (logger.isInfoEnabled()) { logger.info("Configuring " + this.getClass().getSimpleName() + " for '" + context.getProperty(CONNECTION_FACTORY_IMPL).evaluateAttributeExpressions().getValue() + "' to be connected to '" -+ BROKER_URI + "'"); ++ context.getProperty(BROKER_URI).evaluateAttributeExpressions().getValue() + "'"); } + // will load user provided libraries/resources on the classpath - Utils.addResourcesToClasspath(context.getProperty(CLIENT_LIB_DIR_PATH).evaluateAttributeExpressions().getValue()); +final String clientLibPath = context.getProperty(CLIENT_LIB_DIR_PATH).evaluateAttributeExpressions().getValue(); +ClassLoader customClassLoader = ClassLoaderUtils.getCustomClassLoader(clientLibPath, this.getClass().getClassLoader(), null); + Thread.currentThread().setContextClassLoader(customClassLoader); --- End diff -- Understood. I have updated `CLIENT_LIB_DIR_PATH` to include `.dynamicallyModifiesClasspath(true)` > JMSConnectionFactory doesn't resolve 'variables' properly > - > > Key: NIFI-5073 > URL: https://issues.apache.org/jira/browse/NIFI-5073 > Project: Apache NiFi > Issue Type: Bug > Components: Core Framework >Affects Versions: 1.5.0, 1.6.0 >Reporter: Matthew Clarke >Assignee: Sivaprasanna Sethuraman >Priority: Major > Attachments: > 0001-NIFI-5073-JMSConnectionFactoryProvider-now-resolves-.patch > > > Create a new process Group. > Add "Variables" to the process group: > for example: > broker_uri=tcp://localhost:4141 > client_libs=/NiFi/custom-lib-dir/MQlib > con_factory=blah > Then while that process group is selected, create a controller service. > Create JMSConnectionFactory. > Configure this controller service to use EL for PG defined variables above: > ${con_factory}, ${con_factory}, and ${broker_uri} > The controller service will remain invalid because the EL statements are not > properly resolved to their set values. > Doing the exact same thing above using the external NiFi registry file works > as expected. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (NIFI-5073) JMSConnectionFactory doesn't resolve 'variables' properly
[ https://issues.apache.org/jira/browse/NIFI-5073?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16469247#comment-16469247 ] ASF GitHub Bot commented on NIFI-5073: -- Github user markap14 commented on a diff in the pull request: https://github.com/apache/nifi/pull/2653#discussion_r187131152 --- Diff: nifi-nar-bundles/nifi-jms-bundle/nifi-jms-processors/src/main/java/org/apache/nifi/jms/cf/JMSConnectionFactoryProvider.java --- @@ -159,13 +159,15 @@ public void enable(ConfigurationContext context) throws InitializationException if (logger.isInfoEnabled()) { logger.info("Configuring " + this.getClass().getSimpleName() + " for '" + context.getProperty(CONNECTION_FACTORY_IMPL).evaluateAttributeExpressions().getValue() + "' to be connected to '" -+ BROKER_URI + "'"); ++ context.getProperty(BROKER_URI).evaluateAttributeExpressions().getValue() + "'"); } + // will load user provided libraries/resources on the classpath - Utils.addResourcesToClasspath(context.getProperty(CLIENT_LIB_DIR_PATH).evaluateAttributeExpressions().getValue()); +final String clientLibPath = context.getProperty(CLIENT_LIB_DIR_PATH).evaluateAttributeExpressions().getValue(); +ClassLoader customClassLoader = ClassLoaderUtils.getCustomClassLoader(clientLibPath, this.getClass().getClassLoader(), null); + Thread.currentThread().setContextClassLoader(customClassLoader); --- End diff -- The problem with this approach I think is that we're now creating the ClassLoader and using it to create the connection factory. However, when we do that, we would need to ensure that any access to the connection factory instance also is performed using the ClassLoader. Since all calls into this Controller Service could come from different threads, I think this is going to cause a problem. I think the typical pattern here is to update the property descriptor of CLIENT_LIB_DIR_PATH to include .dynamicallyModifiesClassPath(true). In this case, the framework will automatically handle creating the appropriate ClassLoader for each instance of the controller service and will also ensure that the appropriate ClassLoader is set when any method on this Controller Service is invoked. > JMSConnectionFactory doesn't resolve 'variables' properly > - > > Key: NIFI-5073 > URL: https://issues.apache.org/jira/browse/NIFI-5073 > Project: Apache NiFi > Issue Type: Bug > Components: Core Framework >Affects Versions: 1.5.0, 1.6.0 >Reporter: Matthew Clarke >Assignee: Sivaprasanna Sethuraman >Priority: Major > Attachments: > 0001-NIFI-5073-JMSConnectionFactoryProvider-now-resolves-.patch > > > Create a new process Group. > Add "Variables" to the process group: > for example: > broker_uri=tcp://localhost:4141 > client_libs=/NiFi/custom-lib-dir/MQlib > con_factory=blah > Then while that process group is selected, create a controller service. > Create JMSConnectionFactory. > Configure this controller service to use EL for PG defined variables above: > ${con_factory}, ${con_factory}, and ${broker_uri} > The controller service will remain invalid because the EL statements are not > properly resolved to their set values. > Doing the exact same thing above using the external NiFi registry file works > as expected. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (NIFI-5073) JMSConnectionFactory doesn't resolve 'variables' properly
[ https://issues.apache.org/jira/browse/NIFI-5073?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16453791#comment-16453791 ] ASF GitHub Bot commented on NIFI-5073: -- Github user zenfenan commented on a diff in the pull request: https://github.com/apache/nifi/pull/2653#discussion_r184338421 --- Diff: nifi-nar-bundles/nifi-jms-bundle/nifi-jms-processors/src/main/java/org/apache/nifi/jms/cf/JMSConnectionFactoryProvider.java --- @@ -97,7 +96,7 @@ .description("Path to the directory with additional resources (i.e., JARs, configuration files etc.) to be added " + "to the classpath. Such resources typically represent target MQ client libraries for the " + "ConnectionFactory implementation.") -.addValidator(new ClientLibValidator()) +.addValidator(StandardValidators.createListValidator(true, true, StandardValidators.createURLorFileValidator())) --- End diff -- Yep. Thanks for pointing out. I have modified the processor to accept a comma separated list of paths that can be added to Classpath and moreover it would leverage `ClassLoaderUtils` thereby avoiding duplicate. > JMSConnectionFactory doesn't resolve 'variables' properly > - > > Key: NIFI-5073 > URL: https://issues.apache.org/jira/browse/NIFI-5073 > Project: Apache NiFi > Issue Type: Bug > Components: Core Framework >Affects Versions: 1.5.0, 1.6.0 >Reporter: Matthew Clarke >Assignee: Sivaprasanna Sethuraman >Priority: Major > Attachments: > 0001-NIFI-5073-JMSConnectionFactoryProvider-now-resolves-.patch > > > Create a new process Group. > Add "Variables" to the process group: > for example: > broker_uri=tcp://localhost:4141 > client_libs=/NiFi/custom-lib-dir/MQlib > con_factory=blah > Then while that process group is selected, create a controller service. > Create JMSConnectionFactory. > Configure this controller service to use EL for PG defined variables above: > ${con_factory}, ${con_factory}, and ${broker_uri} > The controller service will remain invalid because the EL statements are not > properly resolved to their set values. > Doing the exact same thing above using the external NiFi registry file works > as expected. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (NIFI-5073) JMSConnectionFactory doesn't resolve 'variables' properly
[ https://issues.apache.org/jira/browse/NIFI-5073?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16452929#comment-16452929 ] ASF GitHub Bot commented on NIFI-5073: -- Github user markap14 commented on a diff in the pull request: https://github.com/apache/nifi/pull/2653#discussion_r184179013 --- Diff: nifi-nar-bundles/nifi-jms-bundle/nifi-jms-processors/src/main/java/org/apache/nifi/jms/cf/JMSConnectionFactoryProvider.java --- @@ -97,7 +96,7 @@ .description("Path to the directory with additional resources (i.e., JARs, configuration files etc.) to be added " + "to the classpath. Such resources typically represent target MQ client libraries for the " + "ConnectionFactory implementation.") -.addValidator(new ClientLibValidator()) +.addValidator(StandardValidators.createListValidator(true, true, StandardValidators.createURLorFileValidator())) --- End diff -- I don't think this is the proper validation here. Changing it to support a list of files/directories/urls is probably a good idea. However, at present this processor expects that the configured value be a directory. I think we need to either update the processor to accept the list, or otherwise just use `StandardValidators.createDirectoryExistsValidator(true, false);` > JMSConnectionFactory doesn't resolve 'variables' properly > - > > Key: NIFI-5073 > URL: https://issues.apache.org/jira/browse/NIFI-5073 > Project: Apache NiFi > Issue Type: Bug > Components: Core Framework >Affects Versions: 1.5.0, 1.6.0 >Reporter: Matthew Clarke >Assignee: Sivaprasanna Sethuraman >Priority: Major > Attachments: > 0001-NIFI-5073-JMSConnectionFactoryProvider-now-resolves-.patch > > > Create a new process Group. > Add "Variables" to the process group: > for example: > broker_uri=tcp://localhost:4141 > client_libs=/NiFi/custom-lib-dir/MQlib > con_factory=blah > Then while that process group is selected, create a controller service. > Create JMSConnectionFactory. > Configure this controller service to use EL for PG defined variables above: > ${con_factory}, ${con_factory}, and ${broker_uri} > The controller service will remain invalid because the EL statements are not > properly resolved to their set values. > Doing the exact same thing above using the external NiFi registry file works > as expected. -- This message was sent by Atlassian JIRA (v7.6.3#76005)