You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by GitBox <gi...@apache.org> on 2022/12/05 01:20:36 UTC

[GitHub] [nifi] Lehel44 opened a new pull request, #6756: NIFI-10945: Create a PostgreSQL Controller Service

Lehel44 opened a new pull request, #6756:
URL: https://github.com/apache/nifi/pull/6756

   <!-- Licensed to the Apache Software Foundation (ASF) under one or more -->
   <!-- contributor license agreements.  See the NOTICE file distributed with -->
   <!-- this work for additional information regarding copyright ownership. -->
   <!-- The ASF licenses this file to You under the Apache License, Version 2.0 -->
   <!-- (the "License"); you may not use this file except in compliance with -->
   <!-- the License.  You may obtain a copy of the License at -->
   <!--     http://www.apache.org/licenses/LICENSE-2.0 -->
   <!-- Unless required by applicable law or agreed to in writing, software -->
   <!-- distributed under the License is distributed on an "AS IS" BASIS, -->
   <!-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -->
   <!-- See the License for the specific language governing permissions and -->
   <!-- limitations under the License. -->
   
   # Summary
   
   [NIFI-10945](https://issues.apache.org/jira/browse/NIFI-10945)
   
   # Tracking
   
   Please complete the following tracking steps prior to pull request creation.
   
   ### Issue Tracking
   
   - [ ] [Apache NiFi Jira](https://issues.apache.org/jira/browse/NIFI-10945) issue created
   
   ### Pull Request Tracking
   
   - [ ] Pull Request title starts with Apache NiFi Jira issue number, such as `NIFI-00000`
   - [ ] Pull Request commit message starts with Apache NiFi Jira issue number, as such `NIFI-00000`
   
   ### Pull Request Formatting
   
   - [ ] Pull Request based on current revision of the `main` branch
   - [ ] Pull Request refers to a feature branch with one commit containing changes
   
   # Verification
   
   Please indicate the verification steps performed prior to pull request creation.
   
   ### Build
   
   - [ ] Build completed using `mvn clean install -P contrib-check`
     - [ ] JDK 8
     - [ ] JDK 11
     - [ ] JDK 17
   
   ### Licensing
   
   - [ ] New dependencies are compatible with the [Apache License 2.0](https://apache.org/licenses/LICENSE-2.0) according to the [License Policy](https://www.apache.org/legal/resolved.html)
   - [ ] New dependencies are documented in applicable `LICENSE` and `NOTICE` files
   
   ### Documentation
   
   - [ ] Documentation formatting appears as expected in rendered files
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nifi] bbende commented on a diff in pull request #6756: NIFI-10945: Create a PostgreSQL Controller Service

Posted by GitBox <gi...@apache.org>.
bbende commented on code in PR #6756:
URL: https://github.com/apache/nifi/pull/6756#discussion_r1039710860


##########
nifi-nar-bundles/nifi-dbcp-services-bundle/nifi-postgres-dbcp-service/pom.xml:
##########
@@ -0,0 +1,64 @@
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
+    <!-- Licensed to the Apache Software Foundation (ASF) under one or more
+        contributor license agreements. See the NOTICE file distributed with this
+        work for additional information regarding copyright ownership. The ASF licenses
+        this file to You under the Apache License, Version 2.0 (the "License"); you
+        may not use this file except in compliance with the License. You may obtain
+        a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 Unless
+        required by applicable law or agreed to in writing, software distributed
+        under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES
+        OR CONDITIONS OF ANY KIND, either express or implied. See the License for
+        the specific language governing permissions and limitations under the License. -->
+    <modelVersion>4.0.0</modelVersion>
+    <parent>
+        <groupId>org.apache.nifi</groupId>
+        <artifactId>nifi-dbcp-services-bundle</artifactId>
+        <version>1.19.0-SNAPSHOT</version>
+    </parent>
+
+    <artifactId>nifi-postgres-dbcp-service</artifactId>
+    <packaging>jar</packaging>
+
+    <dependencies>
+        <dependency>
+            <groupId>org.apache.nifi</groupId>
+            <artifactId>nifi-api</artifactId>
+            <version>1.19.0-SNAPSHOT</version>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.nifi</groupId>
+            <artifactId>nifi-dbcp-service-api</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.nifi</groupId>
+            <artifactId>nifi-dbcp-base</artifactId>
+            <version>1.19.0-SNAPSHOT</version>
+            <exclusions>
+                <exclusion>
+                    <groupId>org.apache.nifi</groupId>
+                    <artifactId>nifi-utils</artifactId>
+                </exclusion>
+            </exclusions>
+        </dependency>
+        <dependency>
+            <groupId>org.postgresql</groupId>
+            <artifactId>postgresql</artifactId>
+            <version>42.5.1</version>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.nifi</groupId>
+            <artifactId>nifi-kerberos-credentials-service-api</artifactId>
+            <scope>compile</scope>

Review Comment:
   Any service API dependencies should be `provided` scope since they will be made available through the NAR dependency at runtime.



##########
nifi-nar-bundles/nifi-dbcp-services-bundle/nifi-postgres-dbcp-service/pom.xml:
##########
@@ -0,0 +1,64 @@
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
+    <!-- Licensed to the Apache Software Foundation (ASF) under one or more
+        contributor license agreements. See the NOTICE file distributed with this
+        work for additional information regarding copyright ownership. The ASF licenses
+        this file to You under the Apache License, Version 2.0 (the "License"); you
+        may not use this file except in compliance with the License. You may obtain
+        a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 Unless
+        required by applicable law or agreed to in writing, software distributed
+        under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES
+        OR CONDITIONS OF ANY KIND, either express or implied. See the License for
+        the specific language governing permissions and limitations under the License. -->
+    <modelVersion>4.0.0</modelVersion>
+    <parent>
+        <groupId>org.apache.nifi</groupId>
+        <artifactId>nifi-dbcp-services-bundle</artifactId>
+        <version>1.19.0-SNAPSHOT</version>
+    </parent>
+
+    <artifactId>nifi-postgres-dbcp-service</artifactId>
+    <packaging>jar</packaging>
+
+    <dependencies>
+        <dependency>
+            <groupId>org.apache.nifi</groupId>
+            <artifactId>nifi-api</artifactId>
+            <version>1.19.0-SNAPSHOT</version>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.nifi</groupId>
+            <artifactId>nifi-dbcp-service-api</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.nifi</groupId>
+            <artifactId>nifi-dbcp-base</artifactId>
+            <version>1.19.0-SNAPSHOT</version>
+            <exclusions>
+                <exclusion>
+                    <groupId>org.apache.nifi</groupId>
+                    <artifactId>nifi-utils</artifactId>
+                </exclusion>
+            </exclusions>
+        </dependency>
+        <dependency>
+            <groupId>org.postgresql</groupId>
+            <artifactId>postgresql</artifactId>
+            <version>42.5.1</version>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.nifi</groupId>
+            <artifactId>nifi-kerberos-credentials-service-api</artifactId>
+            <scope>compile</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.nifi</groupId>
+            <artifactId>nifi-kerberos-user-service-api</artifactId>
+            <scope>compile</scope>

Review Comment:
   Same as above



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nifi] exceptionfactory commented on a diff in pull request #6756: NIFI-10945: Create a PostgreSQL Controller Service

Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on code in PR #6756:
URL: https://github.com/apache/nifi/pull/6756#discussion_r1043816292


##########
nifi-nar-bundles/nifi-dbcp-services-bundle/nifi-postgres-dbcp-service/src/main/java/org/apache/nifi/service/PostgreSQLConnectionPool.java:
##########
@@ -0,0 +1,150 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.service;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.nifi.annotation.behavior.RequiresInstanceClassLoading;
+import org.apache.nifi.annotation.documentation.CapabilityDescription;
+import org.apache.nifi.annotation.documentation.Tags;
+import org.apache.nifi.components.PropertyDescriptor;
+import org.apache.nifi.controller.ConfigurationContext;
+import org.apache.nifi.dbcp.AbstractDBCPConnectionPool;
+import org.apache.nifi.expression.ExpressionLanguageScope;
+import org.apache.nifi.processor.exception.ProcessException;
+import org.apache.nifi.processor.util.StandardValidators;
+
+import java.sql.Driver;
+import java.sql.DriverManager;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+
+import static org.apache.nifi.service.JdbcUrlFormat.FULL_URL;
+import static org.apache.nifi.service.JdbcUrlFormat.PARAMETERS;
+import static org.apache.nifi.service.JdbcUrlFormat.POSTGRESQL_BASIC_URI;
+
+/**
+ * Implementation of Database Connection Pooling Service for PostgreSQL with built-in JDBC driver.
+ * Apache DBCP is used for connection pooling functionality.
+ */
+@Tags({"postgres", "postgresql", "dbcp", "jdbc", "database", "connection", "pooling", "store"})
+@CapabilityDescription("Provides PostgreSQL Connection Pooling Service with built-in JDBC driver." +

Review Comment:
   Thanks for highlighting that fact @mattyb149. If the code can be abstracted, this implementation should not support the `SENSITIVE` prefix and should instead support sensitive dynamic properties through the standard annotation.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nifi] mattyb149 commented on a diff in pull request #6756: NIFI-10945: Create a PostgreSQL Controller Service

Posted by GitBox <gi...@apache.org>.
mattyb149 commented on code in PR #6756:
URL: https://github.com/apache/nifi/pull/6756#discussion_r1044090927


##########
nifi-nar-bundles/nifi-dbcp-services-bundle/nifi-postgres-dbcp-service/src/main/java/org/apache/nifi/service/PostgreSQLConnectionPool.java:
##########
@@ -0,0 +1,150 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.service;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.nifi.annotation.behavior.RequiresInstanceClassLoading;
+import org.apache.nifi.annotation.documentation.CapabilityDescription;
+import org.apache.nifi.annotation.documentation.Tags;
+import org.apache.nifi.components.PropertyDescriptor;
+import org.apache.nifi.controller.ConfigurationContext;
+import org.apache.nifi.dbcp.AbstractDBCPConnectionPool;
+import org.apache.nifi.expression.ExpressionLanguageScope;
+import org.apache.nifi.processor.exception.ProcessException;
+import org.apache.nifi.processor.util.StandardValidators;
+
+import java.sql.Driver;
+import java.sql.DriverManager;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+
+import static org.apache.nifi.service.JdbcUrlFormat.FULL_URL;
+import static org.apache.nifi.service.JdbcUrlFormat.PARAMETERS;
+import static org.apache.nifi.service.JdbcUrlFormat.POSTGRESQL_BASIC_URI;
+
+/**
+ * Implementation of Database Connection Pooling Service for PostgreSQL with built-in JDBC driver.
+ * Apache DBCP is used for connection pooling functionality.
+ */
+@Tags({"postgres", "postgresql", "dbcp", "jdbc", "database", "connection", "pooling", "store"})
+@CapabilityDescription("Provides PostgreSQL Connection Pooling Service with built-in JDBC driver." +
+        " Connections can be requested from the pool and returned once they have been used.")
+@RequiresInstanceClassLoading
+public class PostgreSQLConnectionPool extends AbstractDBCPConnectionPool {
+
+    public static final PropertyDescriptor CONNECTION_URL_FORMAT = new PropertyDescriptor.Builder()
+            .name("connection-url-format")
+            .displayName("Connection URL Format")
+            .description("The format of the connection URL.")
+            .allowableValues(JdbcUrlFormat.class)
+            .required(true)
+            .defaultValue(FULL_URL.getValue())
+            .build();
+
+    public static final PropertyDescriptor POSTGRES_DATABASE_URL = new PropertyDescriptor.Builder()
+            .fromPropertyDescriptor(DATABASE_URL)
+            .dependsOn(CONNECTION_URL_FORMAT, FULL_URL)
+            .build();
+
+    public static final PropertyDescriptor DATABASE_NAME = new PropertyDescriptor.Builder()

Review Comment:
   I can see some convenience in the user not having to construct the URL manually, but I think in practice that URL is likely available for use by other tools and such outside of NiFi. Since I haven't heard any pushback about the URL property in DBCPConnectionPool, I tend toward recommending a single way to configure it, namely the full URL, especially if that's the way parameters would be used most often.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nifi] mattyb149 commented on a diff in pull request #6756: NIFI-10945: Create a PostgreSQL Controller Service

Posted by GitBox <gi...@apache.org>.
mattyb149 commented on code in PR #6756:
URL: https://github.com/apache/nifi/pull/6756#discussion_r1043793836


##########
nifi-nar-bundles/nifi-dbcp-services-bundle/nifi-postgres-dbcp-service/src/main/java/org/apache/nifi/service/PostgreSQLConnectionPool.java:
##########
@@ -0,0 +1,150 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.service;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.nifi.annotation.behavior.RequiresInstanceClassLoading;
+import org.apache.nifi.annotation.documentation.CapabilityDescription;
+import org.apache.nifi.annotation.documentation.Tags;
+import org.apache.nifi.components.PropertyDescriptor;
+import org.apache.nifi.controller.ConfigurationContext;
+import org.apache.nifi.dbcp.AbstractDBCPConnectionPool;
+import org.apache.nifi.expression.ExpressionLanguageScope;
+import org.apache.nifi.processor.exception.ProcessException;
+import org.apache.nifi.processor.util.StandardValidators;
+
+import java.sql.Driver;
+import java.sql.DriverManager;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+
+import static org.apache.nifi.service.JdbcUrlFormat.FULL_URL;
+import static org.apache.nifi.service.JdbcUrlFormat.PARAMETERS;
+import static org.apache.nifi.service.JdbcUrlFormat.POSTGRESQL_BASIC_URI;
+
+/**
+ * Implementation of Database Connection Pooling Service for PostgreSQL with built-in JDBC driver.
+ * Apache DBCP is used for connection pooling functionality.
+ */
+@Tags({"postgres", "postgresql", "dbcp", "jdbc", "database", "connection", "pooling", "store"})
+@CapabilityDescription("Provides PostgreSQL Connection Pooling Service with built-in JDBC driver." +
+        " Connections can be requested from the pool and returned once they have been used.")
+@RequiresInstanceClassLoading
+public class PostgreSQLConnectionPool extends AbstractDBCPConnectionPool {
+
+    public static final PropertyDescriptor CONNECTION_URL_FORMAT = new PropertyDescriptor.Builder()
+            .name("connection-url-format")
+            .displayName("Connection URL Format")
+            .description("The format of the connection URL.")
+            .allowableValues(JdbcUrlFormat.class)
+            .required(true)
+            .defaultValue(FULL_URL.getValue())
+            .build();
+
+    public static final PropertyDescriptor POSTGRES_DATABASE_URL = new PropertyDescriptor.Builder()
+            .fromPropertyDescriptor(DATABASE_URL)
+            .dependsOn(CONNECTION_URL_FORMAT, FULL_URL)
+            .build();
+
+    public static final PropertyDescriptor DATABASE_NAME = new PropertyDescriptor.Builder()

Review Comment:
   Is this specifically requested as a feature, to be able to just populate parts of what will become the JDBC URL? If so I presume it has something to do with parameters and such? If not I wonder if we're ok with just the URL property like in DBCPConnectionPool



##########
nifi-nar-bundles/nifi-dbcp-services-bundle/nifi-postgres-dbcp-service/src/main/java/org/apache/nifi/service/PostgreSQLConnectionPool.java:
##########
@@ -0,0 +1,150 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.service;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.nifi.annotation.behavior.RequiresInstanceClassLoading;
+import org.apache.nifi.annotation.documentation.CapabilityDescription;
+import org.apache.nifi.annotation.documentation.Tags;
+import org.apache.nifi.components.PropertyDescriptor;
+import org.apache.nifi.controller.ConfigurationContext;
+import org.apache.nifi.dbcp.AbstractDBCPConnectionPool;
+import org.apache.nifi.expression.ExpressionLanguageScope;
+import org.apache.nifi.processor.exception.ProcessException;
+import org.apache.nifi.processor.util.StandardValidators;
+
+import java.sql.Driver;
+import java.sql.DriverManager;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+
+import static org.apache.nifi.service.JdbcUrlFormat.FULL_URL;
+import static org.apache.nifi.service.JdbcUrlFormat.PARAMETERS;
+import static org.apache.nifi.service.JdbcUrlFormat.POSTGRESQL_BASIC_URI;
+
+/**
+ * Implementation of Database Connection Pooling Service for PostgreSQL with built-in JDBC driver.
+ * Apache DBCP is used for connection pooling functionality.
+ */
+@Tags({"postgres", "postgresql", "dbcp", "jdbc", "database", "connection", "pooling", "store"})
+@CapabilityDescription("Provides PostgreSQL Connection Pooling Service with built-in JDBC driver." +

Review Comment:
   It looks like inheriting from `AbstractDBCPConnectionPool` adds support for JDBC/driver properties via dynamic properties. If so, the same annotation from DBCPConnectionPool should be added here for documentation purposes:
   
   ```
   @DynamicProperties({
           @DynamicProperty(name = "JDBC property name",
                   value = "JDBC property value",
                   expressionLanguageScope = ExpressionLanguageScope.VARIABLE_REGISTRY,
                   description = "JDBC driver property name and value applied to JDBC connections."),
           @DynamicProperty(name = "SENSITIVE.JDBC property name",
                   value = "JDBC property value",
                   expressionLanguageScope = ExpressionLanguageScope.NONE,
                   description = "JDBC driver property name prefixed with 'SENSITIVE.' handled as a sensitive property.")
   })
   ```



##########
nifi-nar-bundles/nifi-dbcp-services-bundle/nifi-postgres-dbcp-service/pom.xml:
##########
@@ -0,0 +1,64 @@
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
+    <!-- Licensed to the Apache Software Foundation (ASF) under one or more
+        contributor license agreements. See the NOTICE file distributed with this
+        work for additional information regarding copyright ownership. The ASF licenses
+        this file to You under the Apache License, Version 2.0 (the "License"); you
+        may not use this file except in compliance with the License. You may obtain
+        a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 Unless
+        required by applicable law or agreed to in writing, software distributed
+        under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES
+        OR CONDITIONS OF ANY KIND, either express or implied. See the License for
+        the specific language governing permissions and limitations under the License. -->
+    <modelVersion>4.0.0</modelVersion>
+    <parent>
+        <groupId>org.apache.nifi</groupId>
+        <artifactId>nifi-dbcp-services-bundle</artifactId>
+        <version>1.20.0-SNAPSHOT</version>
+    </parent>
+
+    <artifactId>nifi-postgres-dbcp-service</artifactId>
+    <packaging>jar</packaging>
+
+    <dependencies>
+        <dependency>
+            <groupId>org.apache.nifi</groupId>
+            <artifactId>nifi-api</artifactId>
+            <version>1.20.0-SNAPSHOT</version>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.nifi</groupId>
+            <artifactId>nifi-dbcp-service-api</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.nifi</groupId>
+            <artifactId>nifi-dbcp-base</artifactId>
+            <version>1.20.0-SNAPSHOT</version>
+            <exclusions>
+                <exclusion>
+                    <groupId>org.apache.nifi</groupId>
+                    <artifactId>nifi-utils</artifactId>
+                </exclusion>
+            </exclusions>
+        </dependency>
+        <dependency>
+            <groupId>org.postgresql</groupId>
+            <artifactId>postgresql</artifactId>
+            <version>42.5.1</version>

Review Comment:
   I presume that if the user needs a different driver, they would just use DBCPConnectionPool? I'm just concerned about the maintenance aspect of this Controller Service in the case of driver upgrades that break backwards compatibility. If this processor stops working for a customer after upgrade because we are now using a driver version that's "too new", they have to replace this with DBCPConnection Pool. I wonder if we should offer a Driver Location Override property or something just in case?



##########
nifi-nar-bundles/nifi-dbcp-services-bundle/nifi-postgres-dbcp-service/src/main/java/org/apache/nifi/service/PostgreSQLConnectionPool.java:
##########
@@ -0,0 +1,150 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.service;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.nifi.annotation.behavior.RequiresInstanceClassLoading;
+import org.apache.nifi.annotation.documentation.CapabilityDescription;
+import org.apache.nifi.annotation.documentation.Tags;
+import org.apache.nifi.components.PropertyDescriptor;
+import org.apache.nifi.controller.ConfigurationContext;
+import org.apache.nifi.dbcp.AbstractDBCPConnectionPool;
+import org.apache.nifi.expression.ExpressionLanguageScope;
+import org.apache.nifi.processor.exception.ProcessException;
+import org.apache.nifi.processor.util.StandardValidators;
+
+import java.sql.Driver;
+import java.sql.DriverManager;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+
+import static org.apache.nifi.service.JdbcUrlFormat.FULL_URL;
+import static org.apache.nifi.service.JdbcUrlFormat.PARAMETERS;
+import static org.apache.nifi.service.JdbcUrlFormat.POSTGRESQL_BASIC_URI;
+
+/**
+ * Implementation of Database Connection Pooling Service for PostgreSQL with built-in JDBC driver.
+ * Apache DBCP is used for connection pooling functionality.
+ */
+@Tags({"postgres", "postgresql", "dbcp", "jdbc", "database", "connection", "pooling", "store"})
+@CapabilityDescription("Provides PostgreSQL Connection Pooling Service with built-in JDBC driver." +
+        " Connections can be requested from the pool and returned once they have been used.")
+@RequiresInstanceClassLoading
+public class PostgreSQLConnectionPool extends AbstractDBCPConnectionPool {
+
+    public static final PropertyDescriptor CONNECTION_URL_FORMAT = new PropertyDescriptor.Builder()
+            .name("connection-url-format")
+            .displayName("Connection URL Format")
+            .description("The format of the connection URL.")
+            .allowableValues(JdbcUrlFormat.class)
+            .required(true)
+            .defaultValue(FULL_URL.getValue())
+            .build();
+
+    public static final PropertyDescriptor POSTGRES_DATABASE_URL = new PropertyDescriptor.Builder()
+            .fromPropertyDescriptor(DATABASE_URL)
+            .dependsOn(CONNECTION_URL_FORMAT, FULL_URL)
+            .build();
+
+    public static final PropertyDescriptor DATABASE_NAME = new PropertyDescriptor.Builder()
+            .name("database-name")
+            .displayName("Database Name")
+            .description("The name of the database to connect.")
+            .required(false)
+            .dependsOn(CONNECTION_URL_FORMAT, PARAMETERS)
+            .addValidator(StandardValidators.NON_EMPTY_VALIDATOR)
+            .expressionLanguageSupported(ExpressionLanguageScope.VARIABLE_REGISTRY)
+            .build();
+
+    public static final PropertyDescriptor DATABASE_HOSTNAME = new PropertyDescriptor.Builder()
+            .name("database-hostname")
+            .displayName("Database Hostname")
+            .description("The hostname of the database to connect.")
+            .required(false)
+            .dependsOn(CONNECTION_URL_FORMAT, PARAMETERS)
+            .expressionLanguageSupported(ExpressionLanguageScope.VARIABLE_REGISTRY)
+            .build();
+
+    public static final PropertyDescriptor DATABASE_PORT = new PropertyDescriptor.Builder()
+            .name("database-port")
+            .displayName("Database Port")
+            .description("The port of the database to connect.")
+            .required(false)
+            .dependsOn(CONNECTION_URL_FORMAT, PARAMETERS)
+            .expressionLanguageSupported(ExpressionLanguageScope.VARIABLE_REGISTRY)
+            .build();
+
+    private static final List<PropertyDescriptor> PROPERTIES = Collections.unmodifiableList(Arrays.asList(
+            CONNECTION_URL_FORMAT,
+            POSTGRES_DATABASE_URL,
+            DATABASE_NAME,
+            DATABASE_HOSTNAME,
+            DATABASE_PORT,
+            DB_USER,
+            DB_PASSWORD,
+            KERBEROS_USER_SERVICE,
+            MAX_WAIT_TIME,
+            MAX_TOTAL_CONNECTIONS,
+            VALIDATION_QUERY,
+            MIN_IDLE,
+            MAX_IDLE,
+            MAX_CONN_LIFETIME,
+            EVICTION_RUN_PERIOD,
+            MIN_EVICTABLE_IDLE_TIME,
+            SOFT_MIN_EVICTABLE_IDLE_TIME
+    ));
+
+    @Override
+    protected List<PropertyDescriptor> getSupportedPropertyDescriptors() {
+        return PROPERTIES;
+    }
+
+    @Override
+    protected Driver getDriver(final String driverName, final String url) {
+        try {
+            return DriverManager.getDriver(url);
+        } catch (Exception e) {
+            throw new ProcessException("PostgreSQL driver unavailable or incompatible connection URL", e);

Review Comment:
   This should only ever happen if they entered an URL in invalid format (certainly not by entering parameters as we construct the URL for the user), recommend adding a unit test for this.



##########
nifi-nar-bundles/nifi-dbcp-services-bundle/nifi-postgres-dbcp-service/src/main/java/org/apache/nifi/service/PostgreSQLConnectionPool.java:
##########
@@ -0,0 +1,150 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.service;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.nifi.annotation.behavior.RequiresInstanceClassLoading;
+import org.apache.nifi.annotation.documentation.CapabilityDescription;
+import org.apache.nifi.annotation.documentation.Tags;
+import org.apache.nifi.components.PropertyDescriptor;
+import org.apache.nifi.controller.ConfigurationContext;
+import org.apache.nifi.dbcp.AbstractDBCPConnectionPool;
+import org.apache.nifi.expression.ExpressionLanguageScope;
+import org.apache.nifi.processor.exception.ProcessException;
+import org.apache.nifi.processor.util.StandardValidators;
+
+import java.sql.Driver;
+import java.sql.DriverManager;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+
+import static org.apache.nifi.service.JdbcUrlFormat.FULL_URL;
+import static org.apache.nifi.service.JdbcUrlFormat.PARAMETERS;
+import static org.apache.nifi.service.JdbcUrlFormat.POSTGRESQL_BASIC_URI;
+
+/**
+ * Implementation of Database Connection Pooling Service for PostgreSQL with built-in JDBC driver.
+ * Apache DBCP is used for connection pooling functionality.
+ */
+@Tags({"postgres", "postgresql", "dbcp", "jdbc", "database", "connection", "pooling", "store"})
+@CapabilityDescription("Provides PostgreSQL Connection Pooling Service with built-in JDBC driver." +
+        " Connections can be requested from the pool and returned once they have been used.")
+@RequiresInstanceClassLoading
+public class PostgreSQLConnectionPool extends AbstractDBCPConnectionPool {
+
+    public static final PropertyDescriptor CONNECTION_URL_FORMAT = new PropertyDescriptor.Builder()
+            .name("connection-url-format")
+            .displayName("Connection URL Format")
+            .description("The format of the connection URL.")

Review Comment:
   Recommend adding a trivial example here, such as `jdbc:postgresql://localhost:5432` or something 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nifi] Lehel44 commented on a diff in pull request #6756: NIFI-10945: Create a PostgreSQL Controller Service

Posted by GitBox <gi...@apache.org>.
Lehel44 commented on code in PR #6756:
URL: https://github.com/apache/nifi/pull/6756#discussion_r1043837059


##########
nifi-nar-bundles/nifi-dbcp-services-bundle/nifi-postgres-dbcp-service/src/main/java/org/apache/nifi/service/PostgreSQLConnectionPool.java:
##########
@@ -0,0 +1,150 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.service;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.nifi.annotation.behavior.RequiresInstanceClassLoading;
+import org.apache.nifi.annotation.documentation.CapabilityDescription;
+import org.apache.nifi.annotation.documentation.Tags;
+import org.apache.nifi.components.PropertyDescriptor;
+import org.apache.nifi.controller.ConfigurationContext;
+import org.apache.nifi.dbcp.AbstractDBCPConnectionPool;
+import org.apache.nifi.expression.ExpressionLanguageScope;
+import org.apache.nifi.processor.exception.ProcessException;
+import org.apache.nifi.processor.util.StandardValidators;
+
+import java.sql.Driver;
+import java.sql.DriverManager;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+
+import static org.apache.nifi.service.JdbcUrlFormat.FULL_URL;
+import static org.apache.nifi.service.JdbcUrlFormat.PARAMETERS;
+import static org.apache.nifi.service.JdbcUrlFormat.POSTGRESQL_BASIC_URI;
+
+/**
+ * Implementation of Database Connection Pooling Service for PostgreSQL with built-in JDBC driver.
+ * Apache DBCP is used for connection pooling functionality.
+ */
+@Tags({"postgres", "postgresql", "dbcp", "jdbc", "database", "connection", "pooling", "store"})
+@CapabilityDescription("Provides PostgreSQL Connection Pooling Service with built-in JDBC driver." +
+        " Connections can be requested from the pool and returned once they have been used.")
+@RequiresInstanceClassLoading
+public class PostgreSQLConnectionPool extends AbstractDBCPConnectionPool {
+
+    public static final PropertyDescriptor CONNECTION_URL_FORMAT = new PropertyDescriptor.Builder()
+            .name("connection-url-format")
+            .displayName("Connection URL Format")
+            .description("The format of the connection URL.")
+            .allowableValues(JdbcUrlFormat.class)
+            .required(true)
+            .defaultValue(FULL_URL.getValue())
+            .build();
+
+    public static final PropertyDescriptor POSTGRES_DATABASE_URL = new PropertyDescriptor.Builder()
+            .fromPropertyDescriptor(DATABASE_URL)
+            .dependsOn(CONNECTION_URL_FORMAT, FULL_URL)
+            .build();
+
+    public static final PropertyDescriptor DATABASE_NAME = new PropertyDescriptor.Builder()

Review Comment:
   According to [Postgres documentation](https://jdbc.postgresql.org/documentation/use/) there are six ways to specify the connection URL:
   
   - jdbc:postgresql:database
   - jdbc:postgresql:/
   - jdbc:postgresql://host/database
   - jdbc:postgresql://host/
   - jdbc:postgresql://host:port/database
   - jdbc:postgresql://host:port/
   
   Since the URL syntax is different based on which parts (db, host, port) provided with the current approach - URL Parameters - the user doesn't need to bother with the URL syntax. The FULL Url property is useful if it comes from a flowfile property as a parameter.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nifi] exceptionfactory commented on a diff in pull request #6756: NIFI-10945: Create a PostgreSQL Controller Service

Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on code in PR #6756:
URL: https://github.com/apache/nifi/pull/6756#discussion_r1044089839


##########
nifi-nar-bundles/nifi-dbcp-services-bundle/nifi-postgres-dbcp-service/src/main/java/org/apache/nifi/service/PostgreSQLConnectionPool.java:
##########
@@ -0,0 +1,150 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.service;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.nifi.annotation.behavior.RequiresInstanceClassLoading;
+import org.apache.nifi.annotation.documentation.CapabilityDescription;
+import org.apache.nifi.annotation.documentation.Tags;
+import org.apache.nifi.components.PropertyDescriptor;
+import org.apache.nifi.controller.ConfigurationContext;
+import org.apache.nifi.dbcp.AbstractDBCPConnectionPool;
+import org.apache.nifi.expression.ExpressionLanguageScope;
+import org.apache.nifi.processor.exception.ProcessException;
+import org.apache.nifi.processor.util.StandardValidators;
+
+import java.sql.Driver;
+import java.sql.DriverManager;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+
+import static org.apache.nifi.service.JdbcUrlFormat.FULL_URL;
+import static org.apache.nifi.service.JdbcUrlFormat.PARAMETERS;
+import static org.apache.nifi.service.JdbcUrlFormat.POSTGRESQL_BASIC_URI;
+
+/**
+ * Implementation of Database Connection Pooling Service for PostgreSQL with built-in JDBC driver.
+ * Apache DBCP is used for connection pooling functionality.
+ */
+@Tags({"postgres", "postgresql", "dbcp", "jdbc", "database", "connection", "pooling", "store"})
+@CapabilityDescription("Provides PostgreSQL Connection Pooling Service with built-in JDBC driver." +

Review Comment:
   Yes `SupportsSensitiveDynamicProperties` was added to `DBCPConnectionPool` and to the framework in NiFi 1.17.0.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nifi] Lehel44 commented on a diff in pull request #6756: NIFI-10945: Create a PostgreSQL Controller Service

Posted by GitBox <gi...@apache.org>.
Lehel44 commented on code in PR #6756:
URL: https://github.com/apache/nifi/pull/6756#discussion_r1043837059


##########
nifi-nar-bundles/nifi-dbcp-services-bundle/nifi-postgres-dbcp-service/src/main/java/org/apache/nifi/service/PostgreSQLConnectionPool.java:
##########
@@ -0,0 +1,150 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.service;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.nifi.annotation.behavior.RequiresInstanceClassLoading;
+import org.apache.nifi.annotation.documentation.CapabilityDescription;
+import org.apache.nifi.annotation.documentation.Tags;
+import org.apache.nifi.components.PropertyDescriptor;
+import org.apache.nifi.controller.ConfigurationContext;
+import org.apache.nifi.dbcp.AbstractDBCPConnectionPool;
+import org.apache.nifi.expression.ExpressionLanguageScope;
+import org.apache.nifi.processor.exception.ProcessException;
+import org.apache.nifi.processor.util.StandardValidators;
+
+import java.sql.Driver;
+import java.sql.DriverManager;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+
+import static org.apache.nifi.service.JdbcUrlFormat.FULL_URL;
+import static org.apache.nifi.service.JdbcUrlFormat.PARAMETERS;
+import static org.apache.nifi.service.JdbcUrlFormat.POSTGRESQL_BASIC_URI;
+
+/**
+ * Implementation of Database Connection Pooling Service for PostgreSQL with built-in JDBC driver.
+ * Apache DBCP is used for connection pooling functionality.
+ */
+@Tags({"postgres", "postgresql", "dbcp", "jdbc", "database", "connection", "pooling", "store"})
+@CapabilityDescription("Provides PostgreSQL Connection Pooling Service with built-in JDBC driver." +
+        " Connections can be requested from the pool and returned once they have been used.")
+@RequiresInstanceClassLoading
+public class PostgreSQLConnectionPool extends AbstractDBCPConnectionPool {
+
+    public static final PropertyDescriptor CONNECTION_URL_FORMAT = new PropertyDescriptor.Builder()
+            .name("connection-url-format")
+            .displayName("Connection URL Format")
+            .description("The format of the connection URL.")
+            .allowableValues(JdbcUrlFormat.class)
+            .required(true)
+            .defaultValue(FULL_URL.getValue())
+            .build();
+
+    public static final PropertyDescriptor POSTGRES_DATABASE_URL = new PropertyDescriptor.Builder()
+            .fromPropertyDescriptor(DATABASE_URL)
+            .dependsOn(CONNECTION_URL_FORMAT, FULL_URL)
+            .build();
+
+    public static final PropertyDescriptor DATABASE_NAME = new PropertyDescriptor.Builder()

Review Comment:
   According to [Postgres documentation](https://jdbc.postgresql.org/documentation/use/) there are six ways to specify the connection URL:
   
   - jdbc:postgresql:database
   - jdbc:postgresql:/
   - jdbc:postgresql://host/database
   - jdbc:postgresql://host/
   - jdbc:postgresql://host:port/database
   - jdbc:postgresql://host:port/
   
   Since the URL syntax is different based on which parts (db, host, port) provided with the current approach - URL Parameters - the user doesn't need to know the URL syntax. The Full URL property is useful if the value comes from a flowfile attribute for instance.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nifi] turcsanyip commented on pull request #6756: NIFI-10945: Create a PostgreSQL Controller Service

Posted by GitBox <gi...@apache.org>.
turcsanyip commented on PR #6756:
URL: https://github.com/apache/nifi/pull/6756#issuecomment-1337196493

   @Lehel44 Thanks of adding this database-specific connection pool controller service for PostgreSQL!
   Can you please eliminate the RAT and Checkstyle issues first? Please also use the current snapshot version in poms: `1.20.0-SNAPSHOT`


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nifi] Lehel44 commented on a diff in pull request #6756: NIFI-10945: Create a PostgreSQL Controller Service

Posted by GitBox <gi...@apache.org>.
Lehel44 commented on code in PR #6756:
URL: https://github.com/apache/nifi/pull/6756#discussion_r1043838815


##########
nifi-nar-bundles/nifi-dbcp-services-bundle/nifi-postgres-dbcp-service/src/main/java/org/apache/nifi/service/PostgreSQLConnectionPool.java:
##########
@@ -0,0 +1,150 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.service;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.nifi.annotation.behavior.RequiresInstanceClassLoading;
+import org.apache.nifi.annotation.documentation.CapabilityDescription;
+import org.apache.nifi.annotation.documentation.Tags;
+import org.apache.nifi.components.PropertyDescriptor;
+import org.apache.nifi.controller.ConfigurationContext;
+import org.apache.nifi.dbcp.AbstractDBCPConnectionPool;
+import org.apache.nifi.expression.ExpressionLanguageScope;
+import org.apache.nifi.processor.exception.ProcessException;
+import org.apache.nifi.processor.util.StandardValidators;
+
+import java.sql.Driver;
+import java.sql.DriverManager;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+
+import static org.apache.nifi.service.JdbcUrlFormat.FULL_URL;
+import static org.apache.nifi.service.JdbcUrlFormat.PARAMETERS;
+import static org.apache.nifi.service.JdbcUrlFormat.POSTGRESQL_BASIC_URI;
+
+/**
+ * Implementation of Database Connection Pooling Service for PostgreSQL with built-in JDBC driver.
+ * Apache DBCP is used for connection pooling functionality.
+ */
+@Tags({"postgres", "postgresql", "dbcp", "jdbc", "database", "connection", "pooling", "store"})
+@CapabilityDescription("Provides PostgreSQL Connection Pooling Service with built-in JDBC driver." +
+        " Connections can be requested from the pool and returned once they have been used.")
+@RequiresInstanceClassLoading
+public class PostgreSQLConnectionPool extends AbstractDBCPConnectionPool {
+
+    public static final PropertyDescriptor CONNECTION_URL_FORMAT = new PropertyDescriptor.Builder()
+            .name("connection-url-format")
+            .displayName("Connection URL Format")
+            .description("The format of the connection URL.")
+            .allowableValues(JdbcUrlFormat.class)
+            .required(true)
+            .defaultValue(FULL_URL.getValue())
+            .build();
+
+    public static final PropertyDescriptor POSTGRES_DATABASE_URL = new PropertyDescriptor.Builder()
+            .fromPropertyDescriptor(DATABASE_URL)
+            .dependsOn(CONNECTION_URL_FORMAT, FULL_URL)
+            .build();
+
+    public static final PropertyDescriptor DATABASE_NAME = new PropertyDescriptor.Builder()

Review Comment:
   We can also omit the URL Parameter option, and describe the URI types in the JDBC Url Property description if that appears to be a better approach.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nifi] Lehel44 closed pull request #6756: NIFI-10945: Create a PostgreSQL Controller Service

Posted by GitBox <gi...@apache.org>.
Lehel44 closed pull request #6756: NIFI-10945: Create a PostgreSQL Controller Service
URL: https://github.com/apache/nifi/pull/6756


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nifi] exceptionfactory commented on a diff in pull request #6756: NIFI-10945: Create a PostgreSQL Controller Service

Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on code in PR #6756:
URL: https://github.com/apache/nifi/pull/6756#discussion_r1039628605


##########
nifi-nar-bundles/nifi-dbcp-services-bundle/nifi-postgres-dbcp-service/src/main/java/org/apache/nifi/service/PostgreSQLConnectionPool.java:
##########
@@ -0,0 +1,255 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.service;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.nifi.annotation.behavior.RequiresInstanceClassLoading;
+import org.apache.nifi.annotation.documentation.CapabilityDescription;
+import org.apache.nifi.annotation.documentation.Tags;
+import org.apache.nifi.components.PropertyDescriptor;
+import org.apache.nifi.components.ValidationContext;
+import org.apache.nifi.components.ValidationResult;
+import org.apache.nifi.controller.ConfigurationContext;
+import org.apache.nifi.dbcp.AbstractDBCPConnectionPool;
+import org.apache.nifi.dbcp.KerberosContext;
+import org.apache.nifi.expression.ExpressionLanguageScope;
+import org.apache.nifi.kerberos.KerberosCredentialsService;
+import org.apache.nifi.kerberos.KerberosUserService;
+import org.apache.nifi.processor.exception.ProcessException;
+import org.apache.nifi.processor.util.StandardValidators;
+
+import java.sql.Driver;
+import java.sql.DriverManager;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.List;
+
+import static org.apache.nifi.service.JdbcUrlFormat.FULL_URL;
+import static org.apache.nifi.service.JdbcUrlFormat.PARAMETERS;
+import static org.apache.nifi.service.JdbcUrlFormat.POSTGRESQL_BASIC_URI;
+
+/**
+ * Implementation of Database Connection Pooling Service for PostgreSQL with built-in JDBC driver.
+ * Apache DBCP is used for connection pooling functionality.
+ */
+@Tags({"postgres", "postgresql", "dbcp", "jdbc", "database", "connection", "pooling", "store"})
+@CapabilityDescription("Provides PostgreSQL Connection Pooling Service with built-in JDBC driver." +
+        " Connections can be requested from the pool and returned once they have been used.")
+@RequiresInstanceClassLoading
+public class PostgreSQLConnectionPool extends AbstractDBCPConnectionPool {
+
+    public static final PropertyDescriptor CONNECTION_URL_FORMAT = new PropertyDescriptor.Builder()
+            .name("connection-url-format")
+            .displayName("Connection URL Format")
+            .description("The format of the connection URL.")
+            .allowableValues(JdbcUrlFormat.class)
+            .required(true)
+            .defaultValue(FULL_URL.getValue())
+            .build();
+
+    public static final PropertyDescriptor POSTGRES_DATABASE_URL = new PropertyDescriptor.Builder()
+            .fromPropertyDescriptor(DATABASE_URL)
+            .dependsOn(CONNECTION_URL_FORMAT, FULL_URL)
+            .build();
+
+    public static final PropertyDescriptor DATABASE_NAME = new PropertyDescriptor.Builder()
+            .name("database-name")
+            .displayName("Database Name")
+            .description("The name of the database to connect.")
+            .required(false)
+            .dependsOn(CONNECTION_URL_FORMAT, PARAMETERS)
+            .addValidator(StandardValidators.NON_EMPTY_VALIDATOR)
+            .expressionLanguageSupported(ExpressionLanguageScope.VARIABLE_REGISTRY)
+            .build();
+
+    public static final PropertyDescriptor DATABASE_HOSTNAME = new PropertyDescriptor.Builder()
+            .name("database-hostname")
+            .displayName("Database Hostname")
+            .description("The hostname of the database to connect.")
+            .required(false)
+            .dependsOn(CONNECTION_URL_FORMAT, PARAMETERS)
+            .expressionLanguageSupported(ExpressionLanguageScope.VARIABLE_REGISTRY)
+            .build();
+
+    public static final PropertyDescriptor DATABASE_PORT = new PropertyDescriptor.Builder()
+            .name("database-port")
+            .displayName("Database Port")
+            .description("The port of the database to connect.")
+            .required(false)
+            .dependsOn(CONNECTION_URL_FORMAT, PARAMETERS)
+            .expressionLanguageSupported(ExpressionLanguageScope.VARIABLE_REGISTRY)
+            .build();
+
+    public static final PropertyDescriptor KERBEROS_AUTH = new PropertyDescriptor.Builder()
+            .name("kerberos-auth")
+            .displayName("Kerberos Authentication")
+            .description("Use Kerberos authentication.")
+            .allowableValues("true", "false")
+            .defaultValue("false")
+            .build();
+
+    public static final PropertyDescriptor POSTGRES_KERBEROS_USER_SERVICE = new PropertyDescriptor.Builder()
+            .fromPropertyDescriptor(KERBEROS_USER_SERVICE)
+            .dependsOn(KERBEROS_AUTH, "true")
+            .build();
+
+    public static final PropertyDescriptor POSTGRES_KERBEROS_CREDENTIALS_SERVICE = new PropertyDescriptor.Builder()
+            .fromPropertyDescriptor(KERBEROS_CREDENTIALS_SERVICE)
+            .dependsOn(KERBEROS_AUTH, "true")
+            .build();

Review Comment:
   The `KerberosCredentialService` should not be necessary since the `KerberosUserService` provides the most configurable approach to Kerberos authentication.



##########
nifi-nar-bundles/nifi-dbcp-services-bundle/nifi-postgres-dbcp-service/src/main/java/org/apache/nifi/service/PostgreSQLConnectionPool.java:
##########
@@ -0,0 +1,255 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.service;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.nifi.annotation.behavior.RequiresInstanceClassLoading;
+import org.apache.nifi.annotation.documentation.CapabilityDescription;
+import org.apache.nifi.annotation.documentation.Tags;
+import org.apache.nifi.components.PropertyDescriptor;
+import org.apache.nifi.components.ValidationContext;
+import org.apache.nifi.components.ValidationResult;
+import org.apache.nifi.controller.ConfigurationContext;
+import org.apache.nifi.dbcp.AbstractDBCPConnectionPool;
+import org.apache.nifi.dbcp.KerberosContext;
+import org.apache.nifi.expression.ExpressionLanguageScope;
+import org.apache.nifi.kerberos.KerberosCredentialsService;
+import org.apache.nifi.kerberos.KerberosUserService;
+import org.apache.nifi.processor.exception.ProcessException;
+import org.apache.nifi.processor.util.StandardValidators;
+
+import java.sql.Driver;
+import java.sql.DriverManager;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.List;
+
+import static org.apache.nifi.service.JdbcUrlFormat.FULL_URL;
+import static org.apache.nifi.service.JdbcUrlFormat.PARAMETERS;
+import static org.apache.nifi.service.JdbcUrlFormat.POSTGRESQL_BASIC_URI;
+
+/**
+ * Implementation of Database Connection Pooling Service for PostgreSQL with built-in JDBC driver.
+ * Apache DBCP is used for connection pooling functionality.
+ */
+@Tags({"postgres", "postgresql", "dbcp", "jdbc", "database", "connection", "pooling", "store"})
+@CapabilityDescription("Provides PostgreSQL Connection Pooling Service with built-in JDBC driver." +
+        " Connections can be requested from the pool and returned once they have been used.")
+@RequiresInstanceClassLoading
+public class PostgreSQLConnectionPool extends AbstractDBCPConnectionPool {
+
+    public static final PropertyDescriptor CONNECTION_URL_FORMAT = new PropertyDescriptor.Builder()
+            .name("connection-url-format")
+            .displayName("Connection URL Format")
+            .description("The format of the connection URL.")
+            .allowableValues(JdbcUrlFormat.class)
+            .required(true)
+            .defaultValue(FULL_URL.getValue())
+            .build();
+
+    public static final PropertyDescriptor POSTGRES_DATABASE_URL = new PropertyDescriptor.Builder()
+            .fromPropertyDescriptor(DATABASE_URL)
+            .dependsOn(CONNECTION_URL_FORMAT, FULL_URL)
+            .build();
+
+    public static final PropertyDescriptor DATABASE_NAME = new PropertyDescriptor.Builder()
+            .name("database-name")
+            .displayName("Database Name")
+            .description("The name of the database to connect.")
+            .required(false)
+            .dependsOn(CONNECTION_URL_FORMAT, PARAMETERS)
+            .addValidator(StandardValidators.NON_EMPTY_VALIDATOR)
+            .expressionLanguageSupported(ExpressionLanguageScope.VARIABLE_REGISTRY)
+            .build();
+
+    public static final PropertyDescriptor DATABASE_HOSTNAME = new PropertyDescriptor.Builder()
+            .name("database-hostname")
+            .displayName("Database Hostname")
+            .description("The hostname of the database to connect.")
+            .required(false)
+            .dependsOn(CONNECTION_URL_FORMAT, PARAMETERS)
+            .expressionLanguageSupported(ExpressionLanguageScope.VARIABLE_REGISTRY)
+            .build();
+
+    public static final PropertyDescriptor DATABASE_PORT = new PropertyDescriptor.Builder()
+            .name("database-port")
+            .displayName("Database Port")
+            .description("The port of the database to connect.")
+            .required(false)
+            .dependsOn(CONNECTION_URL_FORMAT, PARAMETERS)
+            .expressionLanguageSupported(ExpressionLanguageScope.VARIABLE_REGISTRY)
+            .build();
+
+    public static final PropertyDescriptor KERBEROS_AUTH = new PropertyDescriptor.Builder()
+            .name("kerberos-auth")
+            .displayName("Kerberos Authentication")
+            .description("Use Kerberos authentication.")
+            .allowableValues("true", "false")
+            .defaultValue("false")
+            .build();
+
+    public static final PropertyDescriptor POSTGRES_KERBEROS_USER_SERVICE = new PropertyDescriptor.Builder()
+            .fromPropertyDescriptor(KERBEROS_USER_SERVICE)
+            .dependsOn(KERBEROS_AUTH, "true")
+            .build();
+
+    public static final PropertyDescriptor POSTGRES_KERBEROS_CREDENTIALS_SERVICE = new PropertyDescriptor.Builder()
+            .fromPropertyDescriptor(KERBEROS_CREDENTIALS_SERVICE)
+            .dependsOn(KERBEROS_AUTH, "true")
+            .build();
+
+    public static final PropertyDescriptor POSTGRES_KERBEROS_PRINCIPAL = new PropertyDescriptor.Builder()
+            .fromPropertyDescriptor(KERBEROS_PRINCIPAL)
+            .dependsOn(KERBEROS_AUTH, "true")
+            .build();
+
+    public static final PropertyDescriptor POSTGRES_KERBEROS_PASSWORD = new PropertyDescriptor.Builder()
+            .fromPropertyDescriptor(KERBEROS_PASSWORD)
+            .dependsOn(KERBEROS_AUTH, "true")
+            .build();
+
+    private static final List<PropertyDescriptor> PROPERTIES = Collections.unmodifiableList(Arrays.asList(
+            CONNECTION_URL_FORMAT,
+            POSTGRES_DATABASE_URL,
+            DATABASE_NAME,
+            DATABASE_HOSTNAME,
+            DATABASE_PORT,
+            KERBEROS_AUTH,
+            POSTGRES_KERBEROS_USER_SERVICE,
+            POSTGRES_KERBEROS_CREDENTIALS_SERVICE,
+            POSTGRES_KERBEROS_PRINCIPAL,
+            POSTGRES_KERBEROS_PASSWORD,
+            DB_USER,
+            DB_PASSWORD,
+            MAX_WAIT_TIME,
+            MAX_TOTAL_CONNECTIONS,
+            VALIDATION_QUERY,
+            MIN_IDLE,
+            MAX_IDLE,
+            MAX_CONN_LIFETIME,
+            EVICTION_RUN_PERIOD,
+            MIN_EVICTABLE_IDLE_TIME,
+            SOFT_MIN_EVICTABLE_IDLE_TIME
+    ));
+
+    @Override
+    protected List<PropertyDescriptor> getSupportedPropertyDescriptors() {
+        return PROPERTIES;
+    }
+
+    @Override
+    protected Collection<ValidationResult> customValidate(ValidationContext context) {
+        final List<ValidationResult> results = new ArrayList<>();
+
+        final boolean isKerberosAuth = context.getProperty(KERBEROS_AUTH).asBoolean();
+        if (isKerberosAuth) {
+            final boolean kerberosPrincipalProvided = !StringUtils.isBlank(context.getProperty(POSTGRES_KERBEROS_PRINCIPAL).evaluateAttributeExpressions().getValue());
+            final boolean kerberosPasswordProvided = !StringUtils.isBlank(context.getProperty(POSTGRES_KERBEROS_PASSWORD).getValue());
+
+            if (kerberosPrincipalProvided && !kerberosPasswordProvided) {
+                results.add(new ValidationResult.Builder()
+                        .subject(POSTGRES_KERBEROS_PASSWORD.getDisplayName())
+                        .valid(false)
+                        .explanation("a password must be provided for the given principal")
+                        .build());
+            }
+
+            if (kerberosPasswordProvided && !kerberosPrincipalProvided) {
+                results.add(new ValidationResult.Builder()
+                        .subject(POSTGRES_KERBEROS_PRINCIPAL.getDisplayName())
+                        .valid(false)
+                        .explanation("a principal must be provided for the given password")
+                        .build());
+            }
+
+            final KerberosCredentialsService kerberosCredentialsService = context.getProperty(POSTGRES_KERBEROS_CREDENTIALS_SERVICE).asControllerService(KerberosCredentialsService.class);
+            final KerberosUserService kerberosUserService = context.getProperty(POSTGRES_KERBEROS_USER_SERVICE).asControllerService(KerberosUserService.class);
+
+            if (kerberosCredentialsService != null && (kerberosPrincipalProvided || kerberosPasswordProvided)) {
+                results.add(new ValidationResult.Builder()
+                        .subject(POSTGRES_KERBEROS_CREDENTIALS_SERVICE.getDisplayName())
+                        .valid(false)
+                        .explanation("kerberos principal/password and kerberos credential service cannot be configured at the same time")
+                        .build());
+            }
+
+            if (kerberosUserService != null && (kerberosPrincipalProvided || kerberosPasswordProvided)) {
+                results.add(new ValidationResult.Builder()
+                        .subject(POSTGRES_KERBEROS_USER_SERVICE.getDisplayName())
+                        .valid(false)
+                        .explanation("kerberos principal/password and kerberos user service cannot be configured at the same time")
+                        .build());
+            }
+
+            if (kerberosUserService != null && kerberosCredentialsService != null) {
+                results.add(new ValidationResult.Builder()
+                        .subject(POSTGRES_KERBEROS_USER_SERVICE.getDisplayName())
+                        .valid(false)
+                        .explanation("kerberos user service and kerberos credential service cannot be configured at the same time")
+                        .build());
+            }
+        }
+        return results;
+    }
+
+    @Override
+    protected Driver getDriver(final String driverName, final String url) {
+        try {
+            Class.forName(org.postgresql.Driver.class.getName());
+            return DriverManager.getDriver(url);

Review Comment:
   Instead of using this approach, it should be possible to return an instance of the PostgreSQL driver directly.



##########
nifi-nar-bundles/nifi-dbcp-services-bundle/nifi-postgres-dbcp-service/src/main/java/org/apache/nifi/service/PostgreSQLConnectionPool.java:
##########
@@ -0,0 +1,255 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.service;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.nifi.annotation.behavior.RequiresInstanceClassLoading;
+import org.apache.nifi.annotation.documentation.CapabilityDescription;
+import org.apache.nifi.annotation.documentation.Tags;
+import org.apache.nifi.components.PropertyDescriptor;
+import org.apache.nifi.components.ValidationContext;
+import org.apache.nifi.components.ValidationResult;
+import org.apache.nifi.controller.ConfigurationContext;
+import org.apache.nifi.dbcp.AbstractDBCPConnectionPool;
+import org.apache.nifi.dbcp.KerberosContext;
+import org.apache.nifi.expression.ExpressionLanguageScope;
+import org.apache.nifi.kerberos.KerberosCredentialsService;
+import org.apache.nifi.kerberos.KerberosUserService;
+import org.apache.nifi.processor.exception.ProcessException;
+import org.apache.nifi.processor.util.StandardValidators;
+
+import java.sql.Driver;
+import java.sql.DriverManager;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.List;
+
+import static org.apache.nifi.service.JdbcUrlFormat.FULL_URL;
+import static org.apache.nifi.service.JdbcUrlFormat.PARAMETERS;
+import static org.apache.nifi.service.JdbcUrlFormat.POSTGRESQL_BASIC_URI;
+
+/**
+ * Implementation of Database Connection Pooling Service for PostgreSQL with built-in JDBC driver.
+ * Apache DBCP is used for connection pooling functionality.
+ */
+@Tags({"postgres", "postgresql", "dbcp", "jdbc", "database", "connection", "pooling", "store"})
+@CapabilityDescription("Provides PostgreSQL Connection Pooling Service with built-in JDBC driver." +
+        " Connections can be requested from the pool and returned once they have been used.")
+@RequiresInstanceClassLoading
+public class PostgreSQLConnectionPool extends AbstractDBCPConnectionPool {
+
+    public static final PropertyDescriptor CONNECTION_URL_FORMAT = new PropertyDescriptor.Builder()
+            .name("connection-url-format")
+            .displayName("Connection URL Format")
+            .description("The format of the connection URL.")
+            .allowableValues(JdbcUrlFormat.class)
+            .required(true)
+            .defaultValue(FULL_URL.getValue())
+            .build();
+
+    public static final PropertyDescriptor POSTGRES_DATABASE_URL = new PropertyDescriptor.Builder()
+            .fromPropertyDescriptor(DATABASE_URL)
+            .dependsOn(CONNECTION_URL_FORMAT, FULL_URL)
+            .build();
+
+    public static final PropertyDescriptor DATABASE_NAME = new PropertyDescriptor.Builder()
+            .name("database-name")
+            .displayName("Database Name")
+            .description("The name of the database to connect.")
+            .required(false)
+            .dependsOn(CONNECTION_URL_FORMAT, PARAMETERS)
+            .addValidator(StandardValidators.NON_EMPTY_VALIDATOR)
+            .expressionLanguageSupported(ExpressionLanguageScope.VARIABLE_REGISTRY)
+            .build();
+
+    public static final PropertyDescriptor DATABASE_HOSTNAME = new PropertyDescriptor.Builder()
+            .name("database-hostname")
+            .displayName("Database Hostname")
+            .description("The hostname of the database to connect.")
+            .required(false)
+            .dependsOn(CONNECTION_URL_FORMAT, PARAMETERS)
+            .expressionLanguageSupported(ExpressionLanguageScope.VARIABLE_REGISTRY)
+            .build();
+
+    public static final PropertyDescriptor DATABASE_PORT = new PropertyDescriptor.Builder()
+            .name("database-port")
+            .displayName("Database Port")
+            .description("The port of the database to connect.")
+            .required(false)
+            .dependsOn(CONNECTION_URL_FORMAT, PARAMETERS)
+            .expressionLanguageSupported(ExpressionLanguageScope.VARIABLE_REGISTRY)
+            .build();
+
+    public static final PropertyDescriptor KERBEROS_AUTH = new PropertyDescriptor.Builder()
+            .name("kerberos-auth")
+            .displayName("Kerberos Authentication")
+            .description("Use Kerberos authentication.")
+            .allowableValues("true", "false")
+            .defaultValue("false")
+            .build();
+
+    public static final PropertyDescriptor POSTGRES_KERBEROS_USER_SERVICE = new PropertyDescriptor.Builder()
+            .fromPropertyDescriptor(KERBEROS_USER_SERVICE)
+            .dependsOn(KERBEROS_AUTH, "true")
+            .build();
+
+    public static final PropertyDescriptor POSTGRES_KERBEROS_CREDENTIALS_SERVICE = new PropertyDescriptor.Builder()
+            .fromPropertyDescriptor(KERBEROS_CREDENTIALS_SERVICE)
+            .dependsOn(KERBEROS_AUTH, "true")
+            .build();
+
+    public static final PropertyDescriptor POSTGRES_KERBEROS_PRINCIPAL = new PropertyDescriptor.Builder()
+            .fromPropertyDescriptor(KERBEROS_PRINCIPAL)
+            .dependsOn(KERBEROS_AUTH, "true")
+            .build();
+
+    public static final PropertyDescriptor POSTGRES_KERBEROS_PASSWORD = new PropertyDescriptor.Builder()
+            .fromPropertyDescriptor(KERBEROS_PASSWORD)
+            .dependsOn(KERBEROS_AUTH, "true")
+            .build();

Review Comment:
   These properties should be removed together with `KerberosCredentialService` since `KerberosUserService` provides the same functionality.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nifi] Lehel44 commented on a diff in pull request #6756: NIFI-10945: Create a PostgreSQL Controller Service

Posted by GitBox <gi...@apache.org>.
Lehel44 commented on code in PR #6756:
URL: https://github.com/apache/nifi/pull/6756#discussion_r1039656518


##########
nifi-nar-bundles/nifi-dbcp-services-bundle/nifi-postgres-dbcp-service/src/main/java/org/apache/nifi/service/PostgreSQLConnectionPool.java:
##########
@@ -0,0 +1,255 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.service;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.nifi.annotation.behavior.RequiresInstanceClassLoading;
+import org.apache.nifi.annotation.documentation.CapabilityDescription;
+import org.apache.nifi.annotation.documentation.Tags;
+import org.apache.nifi.components.PropertyDescriptor;
+import org.apache.nifi.components.ValidationContext;
+import org.apache.nifi.components.ValidationResult;
+import org.apache.nifi.controller.ConfigurationContext;
+import org.apache.nifi.dbcp.AbstractDBCPConnectionPool;
+import org.apache.nifi.dbcp.KerberosContext;
+import org.apache.nifi.expression.ExpressionLanguageScope;
+import org.apache.nifi.kerberos.KerberosCredentialsService;
+import org.apache.nifi.kerberos.KerberosUserService;
+import org.apache.nifi.processor.exception.ProcessException;
+import org.apache.nifi.processor.util.StandardValidators;
+
+import java.sql.Driver;
+import java.sql.DriverManager;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.List;
+
+import static org.apache.nifi.service.JdbcUrlFormat.FULL_URL;
+import static org.apache.nifi.service.JdbcUrlFormat.PARAMETERS;
+import static org.apache.nifi.service.JdbcUrlFormat.POSTGRESQL_BASIC_URI;
+
+/**
+ * Implementation of Database Connection Pooling Service for PostgreSQL with built-in JDBC driver.
+ * Apache DBCP is used for connection pooling functionality.
+ */
+@Tags({"postgres", "postgresql", "dbcp", "jdbc", "database", "connection", "pooling", "store"})
+@CapabilityDescription("Provides PostgreSQL Connection Pooling Service with built-in JDBC driver." +
+        " Connections can be requested from the pool and returned once they have been used.")
+@RequiresInstanceClassLoading
+public class PostgreSQLConnectionPool extends AbstractDBCPConnectionPool {
+
+    public static final PropertyDescriptor CONNECTION_URL_FORMAT = new PropertyDescriptor.Builder()
+            .name("connection-url-format")
+            .displayName("Connection URL Format")
+            .description("The format of the connection URL.")
+            .allowableValues(JdbcUrlFormat.class)
+            .required(true)
+            .defaultValue(FULL_URL.getValue())
+            .build();
+
+    public static final PropertyDescriptor POSTGRES_DATABASE_URL = new PropertyDescriptor.Builder()
+            .fromPropertyDescriptor(DATABASE_URL)
+            .dependsOn(CONNECTION_URL_FORMAT, FULL_URL)
+            .build();
+
+    public static final PropertyDescriptor DATABASE_NAME = new PropertyDescriptor.Builder()
+            .name("database-name")
+            .displayName("Database Name")
+            .description("The name of the database to connect.")
+            .required(false)
+            .dependsOn(CONNECTION_URL_FORMAT, PARAMETERS)
+            .addValidator(StandardValidators.NON_EMPTY_VALIDATOR)
+            .expressionLanguageSupported(ExpressionLanguageScope.VARIABLE_REGISTRY)
+            .build();
+
+    public static final PropertyDescriptor DATABASE_HOSTNAME = new PropertyDescriptor.Builder()
+            .name("database-hostname")
+            .displayName("Database Hostname")
+            .description("The hostname of the database to connect.")
+            .required(false)
+            .dependsOn(CONNECTION_URL_FORMAT, PARAMETERS)
+            .expressionLanguageSupported(ExpressionLanguageScope.VARIABLE_REGISTRY)
+            .build();
+
+    public static final PropertyDescriptor DATABASE_PORT = new PropertyDescriptor.Builder()
+            .name("database-port")
+            .displayName("Database Port")
+            .description("The port of the database to connect.")
+            .required(false)
+            .dependsOn(CONNECTION_URL_FORMAT, PARAMETERS)
+            .expressionLanguageSupported(ExpressionLanguageScope.VARIABLE_REGISTRY)
+            .build();
+
+    public static final PropertyDescriptor KERBEROS_AUTH = new PropertyDescriptor.Builder()
+            .name("kerberos-auth")
+            .displayName("Kerberos Authentication")
+            .description("Use Kerberos authentication.")
+            .allowableValues("true", "false")
+            .defaultValue("false")
+            .build();
+
+    public static final PropertyDescriptor POSTGRES_KERBEROS_USER_SERVICE = new PropertyDescriptor.Builder()
+            .fromPropertyDescriptor(KERBEROS_USER_SERVICE)
+            .dependsOn(KERBEROS_AUTH, "true")
+            .build();
+
+    public static final PropertyDescriptor POSTGRES_KERBEROS_CREDENTIALS_SERVICE = new PropertyDescriptor.Builder()
+            .fromPropertyDescriptor(KERBEROS_CREDENTIALS_SERVICE)
+            .dependsOn(KERBEROS_AUTH, "true")
+            .build();

Review Comment:
   That's a good idea, thanks!



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nifi] mattyb149 commented on a diff in pull request #6756: NIFI-10945: Create a PostgreSQL Controller Service

Posted by GitBox <gi...@apache.org>.
mattyb149 commented on code in PR #6756:
URL: https://github.com/apache/nifi/pull/6756#discussion_r1044089270


##########
nifi-nar-bundles/nifi-dbcp-services-bundle/nifi-postgres-dbcp-service/src/main/java/org/apache/nifi/service/PostgreSQLConnectionPool.java:
##########
@@ -0,0 +1,150 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.service;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.nifi.annotation.behavior.RequiresInstanceClassLoading;
+import org.apache.nifi.annotation.documentation.CapabilityDescription;
+import org.apache.nifi.annotation.documentation.Tags;
+import org.apache.nifi.components.PropertyDescriptor;
+import org.apache.nifi.controller.ConfigurationContext;
+import org.apache.nifi.dbcp.AbstractDBCPConnectionPool;
+import org.apache.nifi.expression.ExpressionLanguageScope;
+import org.apache.nifi.processor.exception.ProcessException;
+import org.apache.nifi.processor.util.StandardValidators;
+
+import java.sql.Driver;
+import java.sql.DriverManager;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+
+import static org.apache.nifi.service.JdbcUrlFormat.FULL_URL;
+import static org.apache.nifi.service.JdbcUrlFormat.PARAMETERS;
+import static org.apache.nifi.service.JdbcUrlFormat.POSTGRESQL_BASIC_URI;
+
+/**
+ * Implementation of Database Connection Pooling Service for PostgreSQL with built-in JDBC driver.
+ * Apache DBCP is used for connection pooling functionality.
+ */
+@Tags({"postgres", "postgresql", "dbcp", "jdbc", "database", "connection", "pooling", "store"})
+@CapabilityDescription("Provides PostgreSQL Connection Pooling Service with built-in JDBC driver." +

Review Comment:
   Is that a new annotation? Curious to see how that would/should/could affect the existing DBCPConnectionPool



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [nifi] exceptionfactory commented on a diff in pull request #6756: NIFI-10945: Create a PostgreSQL Controller Service

Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on code in PR #6756:
URL: https://github.com/apache/nifi/pull/6756#discussion_r1043816820


##########
nifi-nar-bundles/nifi-dbcp-services-bundle/nifi-postgres-dbcp-service/src/main/java/org/apache/nifi/service/PostgreSQLConnectionPool.java:
##########
@@ -0,0 +1,150 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.service;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.nifi.annotation.behavior.RequiresInstanceClassLoading;
+import org.apache.nifi.annotation.documentation.CapabilityDescription;
+import org.apache.nifi.annotation.documentation.Tags;
+import org.apache.nifi.components.PropertyDescriptor;
+import org.apache.nifi.controller.ConfigurationContext;
+import org.apache.nifi.dbcp.AbstractDBCPConnectionPool;
+import org.apache.nifi.expression.ExpressionLanguageScope;
+import org.apache.nifi.processor.exception.ProcessException;
+import org.apache.nifi.processor.util.StandardValidators;
+
+import java.sql.Driver;
+import java.sql.DriverManager;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+
+import static org.apache.nifi.service.JdbcUrlFormat.FULL_URL;
+import static org.apache.nifi.service.JdbcUrlFormat.PARAMETERS;
+import static org.apache.nifi.service.JdbcUrlFormat.POSTGRESQL_BASIC_URI;
+
+/**
+ * Implementation of Database Connection Pooling Service for PostgreSQL with built-in JDBC driver.
+ * Apache DBCP is used for connection pooling functionality.
+ */
+@Tags({"postgres", "postgresql", "dbcp", "jdbc", "database", "connection", "pooling", "store"})
+@CapabilityDescription("Provides PostgreSQL Connection Pooling Service with built-in JDBC driver." +
+        " Connections can be requested from the pool and returned once they have been used.")
+@RequiresInstanceClassLoading
+public class PostgreSQLConnectionPool extends AbstractDBCPConnectionPool {
+
+    public static final PropertyDescriptor CONNECTION_URL_FORMAT = new PropertyDescriptor.Builder()
+            .name("connection-url-format")
+            .displayName("Connection URL Format")
+            .description("The format of the connection URL.")
+            .allowableValues(JdbcUrlFormat.class)
+            .required(true)
+            .defaultValue(FULL_URL.getValue())
+            .build();
+
+    public static final PropertyDescriptor POSTGRES_DATABASE_URL = new PropertyDescriptor.Builder()
+            .fromPropertyDescriptor(DATABASE_URL)
+            .dependsOn(CONNECTION_URL_FORMAT, FULL_URL)
+            .build();
+
+    public static final PropertyDescriptor DATABASE_NAME = new PropertyDescriptor.Builder()

Review Comment:
   I agree with this concern, having multiple ways to configure the JDBC URL seems to make the overall configuration more confusing.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org