[GitHub] nifi issue #1450: NIFI-3339b Add getDataSource() to DBCPService, second vers...
Github user mattyb149 commented on the issue: https://github.com/apache/nifi/pull/1450 Agreed, sounds good thanks! --- 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 #1450: NIFI-3339b Add getDataSource() to DBCPService, second vers...
Github user ToivoAdams commented on the issue: https://github.com/apache/nifi/pull/1450 @ijokarumawak @mattyb149 We could add example how to use String JDBC to wiki. No need to be part of NiFi codebase. --- 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 #1450: NIFI-3339b Add getDataSource() to DBCPService, second vers...
Github user mattyb149 commented on the issue: https://github.com/apache/nifi/pull/1450 My use case is a third-party library that takes the same information you might put into a DBCPControllerService, either in the URL and/or the username/password, etc. I was hoping to avoid trying to parse the URL looking for some properties (hostname:port) and need access to other properties (username/password). I could just add these properties explicitly to my processor(s), especially if there'd be a lot of refactor involved with making such information available via DBCPService. Your getTransitUri() addition would be a good improvement IMO, I added something similar to HiveDBCPService (a getConnectionURL() method). If we added that to DBCPService we could remove it from HiveDBCPService (although I would still keep the class as a marker interface). I withdraw my use case from this discussion as I can accomplish it another way, and as the processor(s) I am working on are TriggerSerially and to be run on the Primary Node only, I doubt there's much to be gained by reusing a Controller Service anyway. --- 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 #1450: NIFI-3339b Add getDataSource() to DBCPService, second vers...
Github user ijokarumawak commented on the issue:
https://github.com/apache/nifi/pull/1450
@mattyb149 Thanks for sharing your use case. That's interesting.
Why does your processor need DBCPService's URL? For provenance? If so, I'd
prefer creating another interface looks like below:
```java
public interface ExternalSource {
String getTransitUri();
}
```
Then add the interface to ControllerServices that interact with external
source such as DBCPService, so that processors use those controller services
can create more meaningful provenance events. For DBCPService,
DatabaseConnectionURL would be legitimate for a return value of getTransitUri
method.
Or, if you need to use the same configuration among ControllerService and
Processor, then VariableRegistry can be helpful.
How do you think?
---
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 #1450: NIFI-3339b Add getDataSource() to DBCPService, second vers...
Github user mattyb149 commented on the issue: https://github.com/apache/nifi/pull/1450 Koji has a good point about managing the actual DataSource instance internally, if your use case can be accomplished without needing access to this object, then I would tend to agree that we shouldn't expose it. Having said that, I have a use case where I need to get at some of the properties of the controller service. Processors get a ProcessContext passed into some of their methods (like onTrigger) and can use that to get PropertyValues. The ControllerService interface extends ConfigurableComponent (not ConfiguredComponent) so I can currently only get access to the PropertyDescriptors, not the PropertyValues themselves. A compromise might be to expose getter methods from DBCPService to retrieve things like JDBC URL, username, password, etc. However if this could turn into a common pattern for controller services, we should investigate a framework- or API-level solution, such as a getProperty() method added to ControllerService or something. What do you think? --- 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 #1450: NIFI-3339b Add getDataSource() to DBCPService, second vers...
Github user ijokarumawak commented on the issue: https://github.com/apache/nifi/pull/1450 Hi @ToivoAdams I think we should think who should manage the DataSource instance. For me, that should be NiFi DBCPService, because it provides control via NiFi UI or REST API to users. If we expose the DataSource instance itself, other components can alter or close the data source and it can cause unexpected behavior. For example, a custom processor gets data source from DBCPService, then the processor closes the data source unintentionally, then all other processors depending on the same DBCPService will stop working. As for wrapping DBCPService outside, as long as it provides what JdbcTemplate needs, then it should be a right solution. Wrapping it outside of NiFi is nothing wrong to do. On the other hand, removing a method from NiFi framework is harder than adding it. We need to have a good reason to add one to maintain it for a long term. If there's anything specific that doesn't work with that approach, it's easier to discuss. Thanks! --- 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 #1450: NIFI-3339b Add getDataSource() to DBCPService, second vers...
Github user ToivoAdams commented on the issue: https://github.com/apache/nifi/pull/1450 Hi @ijokarumawak I have something like this already in use. This was meant to be temporary solution until ârightâ implementation is available. Now I am little bit confused. Wrapping outside of the DBCPService feels like temporary workaround? No? And just to be sure I need to do some more testing. Thanks Toivo --- 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 #1450: NIFI-3339b Add getDataSource() to DBCPService, second vers...
Github user ijokarumawak commented on the issue:
https://github.com/apache/nifi/pull/1450
Hi @ToivoAdams , thank you for your contribution. I reviewed it and saw the
sample code using Spring JDBC template. Then I felt that this can be done
outside of DBCPService. Adding getDataSource to DBCPService would be overkill.
Instead of adding getDataSource method, how about adding a test method in
DBCPServiceTest like below:
```java
/**
* Test database queries using Derby through Spring JDBC template.
* Connect, create table, insert, select, drop table.
* This is more of an example to use DBCPService with Spring JDBC.
*/
@Test
public void testSpringJDBCTemplate() throws InitializationException,
SQLException {
final TestRunner runner =
TestRunners.newTestRunner(TestProcessor.class);
final DBCPConnectionPool service = new DBCPConnectionPool();
runner.addControllerService("test-good1", service);
// remove previous test database, if any
final File dbLocation = new File(DB_LOCATION);
dbLocation.delete();
// set embedded Derby database connection url
runner.setProperty(service, DBCPConnectionPool.DATABASE_URL,
"jdbc:derby:" + DB_LOCATION + ";create=true");
runner.setProperty(service, DBCPConnectionPool.DB_USER, "tester");
runner.setProperty(service, DBCPConnectionPool.DB_PASSWORD,
"testerp");
runner.setProperty(service, DBCPConnectionPool.DB_DRIVERNAME,
"org.apache.derby.jdbc.EmbeddedDriver");
runner.enableControllerService(service);
runner.assertValid(service);
final DBCPService dbcpService = (DBCPService)
runner.getProcessContext().getControllerServiceLookup().getControllerService("test-good1");
Assert.assertNotNull(dbcpService);
// Create a jdbcTemplate. Wrap dbcpService so that it can act as a
DataSource.
JdbcTemplate jdbcTemplate = new JdbcTemplate(new BasicDataSource() {
@Override
public Connection getConnection() throws SQLException {
return dbcpService.getConnection();
}
@Override
public Connection getConnection(String user, String pass)
throws SQLException {
throw new UnsupportedOperationException("User and password
can not be overwritten.");
}
});
try {
jdbcTemplate.execute(dropTable);
} catch (final Exception e) {
// table may not exist, this is not serious problem.
}
jdbcTemplate.execute(createTable);
jdbcTemplate.update("insert into restaurants values (1, 'Irifunes',
'San Mateo')");
jdbcTemplate.update("insert into restaurants values (2, 'Estradas',
'Daly City')");
jdbcTemplate.update("insert into restaurants values (3, 'Prime Rib
House', 'San Francisco')");
int nrOfRows = jdbcTemplate.queryForObject("select count(*) from
restaurants", Integer.class);
assertEquals(3, nrOfRows);
}
```
This way, we can let other developers know that there're users using
DBCPService in their custom processors integrated with Spring JDBC framework.
Also if DBCPService changes its signature or behavior in the future, we can
detect that breaking change by this test.
How do you think?
---
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.
---
