Details

    • Type: Improvement
    • Status: Patch Available
    • Priority: Minor
    • Resolution: Unresolved
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: Extensions
    • Labels:
      None

      Description

      Currently DBCPService returns only Connection.
      Sometimes DataSource is needed, for example Spring JdbcTemplate, SimpleJdbcCall need DataSource.

        Issue Links

          Activity

          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user ToivoAdams opened a pull request:

          https://github.com/apache/nifi/pull/1417

          NIFI-3339 Add getDataSource() to DBCPService.

          Thank you for submitting a contribution to Apache NiFi.

          In order to streamline the review of the contribution we ask you
          to ensure the following steps have been taken:

              1. For all changes:
          • [ ] Is there a JIRA ticket associated with this PR? Is it referenced
            in the commit message?
          • [ ] Does your PR title start with NIFI-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
          • [ ] Has your PR been rebased against the latest commit within the target branch (typically master)?
          • [ ] Is your initial contribution a single, squashed commit?
              1. For code changes:
          • [ ] Have you ensured that the full suite of tests is executed via mvn -Pcontrib-check clean install at the root nifi folder?
          • [ ] Have you written or updated unit tests to verify your changes?
          • [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
          • [ ] If applicable, have you updated the LICENSE file, including the main LICENSE file under nifi-assembly?
          • [ ] If applicable, have you updated the NOTICE file, including the main NOTICE file found under nifi-assembly?
          • [ ] If adding new Properties, have you added .displayName in addition to .name (programmatic access) for each of the new properties?
              1. For documentation related changes:
          • [ ] Have you ensured that format looks appropriate for the output in which it is rendered?
              1. Note:
                Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible.

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

          $ git pull https://github.com/ToivoAdams/nifi nifi-3339

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

          https://github.com/apache/nifi/pull/1417.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 #1417


          commit fcdc9f9ff716631eaf3a4b18fa9087600683171c
          Author: Toivo Adams <toivo.adams@gmail.com>
          Date: 2017-01-14T10:19:13Z

          NIFI-3339 Add getDataSource() to DBCPService.


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user ToivoAdams opened a pull request: https://github.com/apache/nifi/pull/1417 NIFI-3339 Add getDataSource() to DBCPService. Thank you for submitting a contribution to Apache NiFi. In order to streamline the review of the contribution we ask you to ensure the following steps have been taken: For all changes: [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message? [ ] Does your PR title start with NIFI-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character. [ ] Has your PR been rebased against the latest commit within the target branch (typically master)? [ ] Is your initial contribution a single, squashed commit? For code changes: [ ] Have you ensured that the full suite of tests is executed via mvn -Pcontrib-check clean install at the root nifi folder? [ ] Have you written or updated unit tests to verify your changes? [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0] ( http://www.apache.org/legal/resolved.html#category-a)? [ ] If applicable, have you updated the LICENSE file, including the main LICENSE file under nifi-assembly? [ ] If applicable, have you updated the NOTICE file, including the main NOTICE file found under nifi-assembly? [ ] If adding new Properties, have you added .displayName in addition to .name (programmatic access) for each of the new properties? For documentation related changes: [ ] Have you ensured that format looks appropriate for the output in which it is rendered? Note: Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible. You can merge this pull request into a Git repository by running: $ git pull https://github.com/ToivoAdams/nifi nifi-3339 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/nifi/pull/1417.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 #1417 commit fcdc9f9ff716631eaf3a4b18fa9087600683171c Author: Toivo Adams <toivo.adams@gmail.com> Date: 2017-01-14T10:19:13Z NIFI-3339 Add getDataSource() to DBCPService.
          Hide
          githubbot ASF GitHub Bot added a comment -

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

          https://github.com/apache/nifi/pull/1417#discussion_r97923254

          — Diff: nifi-nar-bundles/nifi-standard-services/nifi-dbcp-service-api/src/main/java/org/apache/nifi/dbcp/DBCPService.java —
          @@ -31,4 +33,6 @@
          @CapabilityDescription("Provides Database Connection Pooling Service. Connections can be asked from pool and returned after usage.")
          public interface DBCPService extends ControllerService {
          public Connection getConnection() throws ProcessException;
          +
          + public DataSource getDataSource() throws ProcessException;
          — End diff –

          I concern a bit about exposing DataSource object, because javax.sql.DataSouce has additional getConnection method that takes username and password. It enables extension points accessing different user/privilege to the data source without NiFi data flow manager's intention.

          Since DataSource interface only has methods to create a Connection, is it possible to create a DBCPService instance as following example code to work with Spring framework? Instead of exposing DataSource instance?

          ```java
          // Acquire DBCPService instance somehow.
          final DBCPService dbcpService = null;

          // Create DataSource instance extending BasicDataSource which returns connection created by service.
          final DataSource dataSource = new BasicDataSource() {
          @Override
          public Connection getConnection() throws SQLException

          { return dbcpService.getConnection(); }

          @Override
          public Connection getConnection(String user, String pass) throws SQLException

          { throw new SQLException("User and password can not be overwritten."); }

          };

          // Write code using dataSource instance.
          ```

          Show
          githubbot ASF GitHub Bot added a comment - Github user ijokarumawak commented on a diff in the pull request: https://github.com/apache/nifi/pull/1417#discussion_r97923254 — Diff: nifi-nar-bundles/nifi-standard-services/nifi-dbcp-service-api/src/main/java/org/apache/nifi/dbcp/DBCPService.java — @@ -31,4 +33,6 @@ @CapabilityDescription("Provides Database Connection Pooling Service. Connections can be asked from pool and returned after usage.") public interface DBCPService extends ControllerService { public Connection getConnection() throws ProcessException; + + public DataSource getDataSource() throws ProcessException; — End diff – I concern a bit about exposing DataSource object, because javax.sql.DataSouce has additional getConnection method that takes username and password. It enables extension points accessing different user/privilege to the data source without NiFi data flow manager's intention. Since DataSource interface only has methods to create a Connection, is it possible to create a DBCPService instance as following example code to work with Spring framework? Instead of exposing DataSource instance? ```java // Acquire DBCPService instance somehow. final DBCPService dbcpService = null; // Create DataSource instance extending BasicDataSource which returns connection created by service. final DataSource dataSource = new BasicDataSource() { @Override public Connection getConnection() throws SQLException { return dbcpService.getConnection(); } @Override public Connection getConnection(String user, String pass) throws SQLException { throw new SQLException("User and password can not be overwritten."); } }; // Write code using dataSource instance. ```
          Hide
          githubbot ASF GitHub Bot added a comment -

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

          https://github.com/apache/nifi/pull/1417#discussion_r97921016

          — Diff: nifi-nar-bundles/nifi-standard-services/nifi-dbcp-service-api/src/main/java/org/apache/nifi/dbcp/DBCPService.java —
          @@ -31,4 +33,6 @@
          @CapabilityDescription("Provides Database Connection Pooling Service. Connections can be asked from pool and returned after usage.")
          public interface DBCPService extends ControllerService {
          public Connection getConnection() throws ProcessException;
          +
          + public DataSource getDataSource() throws ProcessException;
          — End diff –

          If we declare getDataSource() method with default implementation like below, we don't have to touch every existing implementation codes in test classes:

          ```
          default DataSource getDataSource() throws ProcessException

          { throw new UnsupportedOperationException(); }

          ```

          Show
          githubbot ASF GitHub Bot added a comment - Github user ijokarumawak commented on a diff in the pull request: https://github.com/apache/nifi/pull/1417#discussion_r97921016 — Diff: nifi-nar-bundles/nifi-standard-services/nifi-dbcp-service-api/src/main/java/org/apache/nifi/dbcp/DBCPService.java — @@ -31,4 +33,6 @@ @CapabilityDescription("Provides Database Connection Pooling Service. Connections can be asked from pool and returned after usage.") public interface DBCPService extends ControllerService { public Connection getConnection() throws ProcessException; + + public DataSource getDataSource() throws ProcessException; — End diff – If we declare getDataSource() method with default implementation like below, we don't have to touch every existing implementation codes in test classes: ``` default DataSource getDataSource() throws ProcessException { throw new UnsupportedOperationException(); } ```
          Hide
          githubbot ASF GitHub Bot added a comment -

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

          https://github.com/apache/nifi/pull/1417#discussion_r97919769

          — Diff: nifi-nar-bundles/nifi-hive-bundle/nifi-hive-processors/src/test/java/org/apache/nifi/processors/hive/TestPutHiveQL.java —
          @@ -493,8 +496,12 @@ public void testRetryableFailure() throws InitializationException, ProcessExcept
          private static class MockDBCPService extends AbstractControllerService implements HiveDBCPService {
          private final String dbLocation;

          + BasicDataSource dataSource = new BasicDataSource();
          +
          — End diff –

          Travis build is failing due to check-style error.

          ```
          [ERROR] Failed to execute goal org.apache.maven.plugins:maven-checkstyle-plugin:2.15:check (check-style) on project nifi-hive-processors: You have 2 Checkstyle violations. -> [Help 1]
          ```

          Please run `mvn -Pcontrib-check clean install` at the root nifi folder to ensure there's no check-style issue.
          You can find error detail in target/checkstyle-result.xml created in each project, looks like below:

          ```
          <file name="/Users/koji/dev/nifi/nifi-nar-bundles/nifi-hive-bundle/nifi-hive-processors/src/test/java/org/apache/nifi/processors/hive/TestPutHiveQL.java">
          <error line="500" severity="warning" message="Line has trailing whitespace." source="com.puppycrawl.tools.checkstyle.checks.regexp.RegexpSinglelineJavaCheck"/>
          </file>
          ```

          Show
          githubbot ASF GitHub Bot added a comment - Github user ijokarumawak commented on a diff in the pull request: https://github.com/apache/nifi/pull/1417#discussion_r97919769 — Diff: nifi-nar-bundles/nifi-hive-bundle/nifi-hive-processors/src/test/java/org/apache/nifi/processors/hive/TestPutHiveQL.java — @@ -493,8 +496,12 @@ public void testRetryableFailure() throws InitializationException, ProcessExcept private static class MockDBCPService extends AbstractControllerService implements HiveDBCPService { private final String dbLocation; + BasicDataSource dataSource = new BasicDataSource(); + — End diff – Travis build is failing due to check-style error. ``` [ERROR] Failed to execute goal org.apache.maven.plugins:maven-checkstyle-plugin:2.15:check (check-style) on project nifi-hive-processors: You have 2 Checkstyle violations. -> [Help 1] ``` Please run `mvn -Pcontrib-check clean install` at the root nifi folder to ensure there's no check-style issue. You can find error detail in target/checkstyle-result.xml created in each project, looks like below: ``` <file name="/Users/koji/dev/nifi/nifi-nar-bundles/nifi-hive-bundle/nifi-hive-processors/src/test/java/org/apache/nifi/processors/hive/TestPutHiveQL.java"> <error line="500" severity="warning" message="Line has trailing whitespace." source="com.puppycrawl.tools.checkstyle.checks.regexp.RegexpSinglelineJavaCheck"/> </file> ```
          Hide
          Toivo Adams Toivo Adams added a comment -

          Thank you for reviewing

          Koji Kawamura Great ideas.
          I created new PR.
          https://github.com/apache/nifi/pull/1450

          Spring JDBCTemplate usage example is in DBCPServiceTest.java
          testGetDataSource() and createInsertSelectDrop(DataSource dataSource)

          Thanks
          Toivo

          Show
          Toivo Adams Toivo Adams added a comment - Thank you for reviewing Koji Kawamura Great ideas. I created new PR. https://github.com/apache/nifi/pull/1450 Spring JDBCTemplate usage example is in DBCPServiceTest.java testGetDataSource() and createInsertSelectDrop(DataSource dataSource) Thanks Toivo
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user ijokarumawak commented on the issue:

          https://github.com/apache/nifi/pull/1417

          @ToivoAdams Would you close this PR as you opened new PR #1450 for the same JIRA. Usually, you just need to add commit to your branch and push it, in order to update a PR which is already submitted.
          https://cwiki.apache.org/confluence/display/NIFI/Contributor+Guide#ContributorGuide-Pushchangestoyourpersonal,GitHubrepositoryremote

          If you'd like to rewrite a PR completely, you can squash commits then push it to your remote branch with '-f' option.
          https://ariejan.net/2011/07/05/git-squash-your-latests-commits-into-one/

          Thanks!

          Show
          githubbot ASF GitHub Bot added a comment - Github user ijokarumawak commented on the issue: https://github.com/apache/nifi/pull/1417 @ToivoAdams Would you close this PR as you opened new PR #1450 for the same JIRA. Usually, you just need to add commit to your branch and push it, in order to update a PR which is already submitted. https://cwiki.apache.org/confluence/display/NIFI/Contributor+Guide#ContributorGuide-Pushchangestoyourpersonal,GitHubrepositoryremote If you'd like to rewrite a PR completely, you can squash commits then push it to your remote branch with '-f' option. https://ariejan.net/2011/07/05/git-squash-your-latests-commits-into-one/ Thanks!
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user ToivoAdams closed the pull request at:

          https://github.com/apache/nifi/pull/1417

          Show
          githubbot ASF GitHub Bot added a comment - Github user ToivoAdams closed the pull request at: https://github.com/apache/nifi/pull/1417

            People

            • Assignee:
              Toivo Adams Toivo Adams
              Reporter:
              Toivo Adams Toivo Adams
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:

                Development