NIFI-15717: Add QueryCassandra processor and CassandraSessionProvider Controller Service.#11032
NIFI-15717: Add QueryCassandra processor and CassandraSessionProvider Controller Service.#11032vishal-firgan-ksolves wants to merge 3 commits intoapache:mainfrom
Conversation
exceptionfactory
left a comment
exceptionfactory
left a comment
There was a problem hiding this comment.
Thanks for putting together this down-scoped pull request. I'm planning to take a closer look soon, but it provides a good basis for initial review.
One initial element to consider is the data type handling in the AbstractCassandraProcessor. It seems like it would be best to pull that out to a separate interface and implementation, perhaps several depending on the details. The general goal there is to decouple type conversion from the Processor itself, so that it can be used in composition, rather than by extension in other Cassandra Processors.
|
@exceptionfactory I’ve implemented the changes to decouple type conversion from AbstractCassandraProcessor into a new CassandraTypeConverter interface with StandardCassandraTypeConverter as its implementation. |
exceptionfactory
left a comment
exceptionfactory
left a comment
There was a problem hiding this comment.
Thanks for the initial update @vishal-firgan-ksolves.
On a more substantive review, I noticed a large number of significant design and implementation issues. I highlighted a number of them, but based on this review, this will require substantial rework before going forward. I'm not in a position to provide direction on every detail, so this may need to be revisited down the road.
| <dependency> | ||
| <groupId>org.apache.nifi</groupId> | ||
| <artifactId>nifi-cassandra-processors</artifactId> | ||
| <version>2.9.0-SNAPSHOT</version> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>org.apache.nifi</groupId> | ||
| <artifactId>nifi-cassandra-services</artifactId> | ||
| <version>2.9.0-SNAPSHOT</version> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>org.apache.nifi</groupId> | ||
| <artifactId>nifi-cassandra-services-api</artifactId> | ||
| <version>2.9.0-SNAPSHOT</version> | ||
| </dependency> |
There was a problem hiding this comment.
Component JAR dependencies do not belong in the nifi-assembly module.
| @@ -0,0 +1,323 @@ | |||
| nifi-cassandra-nar | |||
There was a problem hiding this comment.
Many of the references in this file are not accurate, because they are instead included in shared NAR bundles. Please review each library listed against the actual NAR contents.
| <groupId>org.apache.nifi</groupId> | ||
| <artifactId>nifi-cassandra-bundle</artifactId> | ||
| <version>2.9.0-SNAPSHOT</version> | ||
| <relativePath>../pom.xml</relativePath> |
There was a problem hiding this comment.
This relative path is not needed and should be removed.
| } else if (type.equals(DataTypes.TIME)) { | ||
| return Long.class; | ||
| } else if (type.equals(DataTypes.BLOB)) { | ||
| return java.nio.ByteBuffer.class; |
There was a problem hiding this comment.
Qualified class name references should be avoided in favor of standard imports.
| */ | ||
| public abstract class AbstractCassandraProcessor extends AbstractProcessor { | ||
|
|
||
| static final PropertyDescriptor CONNECTION_PROVIDER_SERVICE = new PropertyDescriptor.Builder() |
There was a problem hiding this comment.
Property Descriptor variables should align closely with the property name, such as CASSANDRA_CONNECTION_PROVIDER.
| See the License for the specific language governing permissions and | ||
| limitations under the License. | ||
|
|
||
| APACHE NIFI SUBCOMPONENTS: |
There was a problem hiding this comment.
This license file is incorrect and contains references that are not applicable.
| .build(); | ||
|
|
||
| static final PropertyDescriptor READ_TIMEOUT_MS = new PropertyDescriptor.Builder() | ||
| .name("Read Timeout (ms)") |
There was a problem hiding this comment.
This convention is not correct, duration validators and syntax should be used.
| # See the License for the specific language governing permissions and | ||
| # limitations under the License. | ||
|
|
||
| org.apache.nifi.service.CassandraSessionProvider No newline at end of file |
There was a problem hiding this comment.
This should be changed to a cassandra subpackage.
| /** | ||
| * Mock Cassandra processor for testing CassandraSessionProvider | ||
| */ | ||
| public class MockCassandraProcessor extends AbstractProcessor { |
There was a problem hiding this comment.
What is the reason for this class, as opposed to just using Mockito?
| <dependency> | ||
| <groupId>org.lz4</groupId> | ||
| <artifactId>lz4-java</artifactId> | ||
| <version>${lz4-java.version}</version> | ||
| </dependency> |
There was a problem hiding this comment.
This dependency is banned.
2 participants
…r Service.
Summary
NIFI-15717
This PR introduces the QueryCassandra processor for Apache NiFi 2.x, enabling integration with Cassandra using the latest DataStax Java driver.
Components Added
Tracking
Please complete the following tracking steps prior to pull request creation.
Issue Tracking
Pull Request Tracking
NIFI-00000NIFI-00000VerifiedstatusPull Request Formatting
mainbranchVerification
Please indicate the verification steps performed prior to pull request creation.
Build
./mvnw clean install -P contrib-checkLicensing
LICENSEandNOTICEfilesDocumentation