Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[NIFI-13408] Dynamically Update Kafka Topic in ConsumeKafka_2_6 Processor #8971

Closed
wants to merge 1 commit into from

Conversation

andrealves23
Copy link
Contributor

Summary

NIFI-13408

Tracking

Please complete the following tracking steps prior to pull request creation.

Issue Tracking

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 21

Licensing

  • New dependencies are compatible with the Apache License 2.0 according to the License Policy
  • New dependencies are documented in applicable LICENSE and NOTICE files

Documentation

  • Documentation formatting appears as expected in rendered files

@exceptionfactory
Copy link
Contributor

Thanks for putting together this pull request @andrealves23. For initial review, can you squash the existing commits into a single a commit? That should remove the merge commits and make it easier to start with a clean base set of changes.

Copy link
Contributor

@exceptionfactory exceptionfactory left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also of note on the substance, I'm not sure about the implications of this particular change, as the Processor is designed as a "source", not to receive input FlowFiles. There is a large refactor in process for Kafka Processors which should be ready to merge soon, so that is something else to consider.

@andrealves23 andrealves23 force-pushed the consumer-kafka-2-6 branch 3 times, most recently from 038991d to 06cebc1 Compare June 17, 2024 15:34
Set sensitive as true in USM Users JSON content property

Trigger CI pipeline

[NIFI-13303] - Remove text indicating property verification is disabled. (apache#8884)

This closes apache#8884

[NIFI-13269] - Order parameter reference list alphabetically (apache#8885)

* [NIFI-13269] - Order parameter reference list alphabetically

* ran prettier:format to address minor code style issue

* update nfpr and nfel to sort combo entries the same as the combo entries in the property table (case insensitive)

This closes apache#8885

NIFI-13284: (apache#8891)

- Only saving canvas routes that are not edit or history.

This closes apache#8891

NIFI-12343 Added Max JSON Field String Length for Elasticsearch

This closes apache#8881

Signed-off-by: David Handermann <exceptionfactory@apache.org>

NIFI-13308 Upgraded Spring Framework from 6.1.7 to 6.1.8

- Upgraded Spring Boot from 3.2.5 to 3.2.6
- Upgraded Slack bolt-socket-mode from 1.39.2 to 1.39.3
- Upgraded maven-artifact from 3.9.6 to 3.9.7
- Upgraded mariadb-java-client from 3.3.3 to 3.4.0
- Upgraded software.amazon.awssdk from 2.25.55 to 2.25.60
- Upgraded com.amazonaws from 1.12.725 to 1.12.730
- Upgraded Jersey from 3.1.6 to 3.1.7
- Upgraded Netty from 4.1.109.Final to 4.1.110.Final
- Upgraded box-java-sdk from 4.9.0 to 4.9.1

This closes apache#8887

Signed-off-by: David Handermann <exceptionfactory@apache.org>

NIFI-13299: (apache#8894)

- Adding min validators where appropriate.

This closes apache#8894

NIFI-13289 add tooltips to NewCanvasItem (apache#8870)

This closes apache#8870

NIFI-13315 Fixed ListAzureBlobStorage_v12 fails when Record Writer is used

This closes apache#8897

Signed-off-by: Mark Bathori <mbathori@apache.org>

[NIFI-13312] - Restructure as an Nx monorepo (apache#8893)

* [NIFI-13312] - Restructure as an Nx monorepo

* restored lint:fix functionality, updated package-lock

This closes apache#8893

[NIFI-13234] update unauthorized canvas component colors (apache#8902)

* [NIFI-13234] update unautorized canvas component colors

* restore web font loader to ensure positions of canvas text is calculate correctly

This closes apache#8902

[NIFI-13246] move actions from details columns into menu (apache#8900)

* [NIFI-13246] move actions from details columns into menu

* move View Documentation menu option lower

This closes apache#8900

NIFI-13321: (apache#8901)

- Fixing the mocking of child components in unit tests.
- Removing any provided dependency that is no longer needed.

This closes apache#8901

NIFI-13265 Removed instantiation of Object arrays for log arguments

This closes apache#8896

Signed-off-by: David Handermann <exceptionfactory@apache.org>

NIFI-13309 Lookup compatible bundles even if previous flow was empty

Signed-off-by: Ferenc Kis <briansolo1985@gmail.com>

This closes apache#8888.

NIFI-13320 Upgraded Spring Boot from 3.2.6 to 3.3.0

Signed-off-by: Pierre Villard <pierre.villard.fr@gmail.com>

This closes apache#8899.

NIFI-13267 - Bump NiFi NAR Maven plugin version (apache#8860)

* NIFI-13267 - Bump NiFi NAR Maven plugin version
* Review - adding Parameter Providers and Flow Analaysis Rules in c2-protocol-component-api ComponentManifest
* Review - fix build() of ComponentManifest

NIFI-13307 Replaced KeyStoreUtils with nifi-security-ssl Builders (apache#8895)

- Removed unused test classes from nifi-web-api

NIFI-13336 updating various deps for aws google azure and more

- com.amazonaws	* 1.12.730 1.12.733
- com.azure azure-sdk-bom 1.2.23 1.2.24
- com.google.cloud libraries-bom 26.39.0 26.40.0
- commons-cli 1.7.0 1.8.0
- commons-net 3.10.0 3.11.0
- io.fabric8 * 6.12.1 6.13.0
- org.apache.commons commons-compress 1.26.1 1.26.2
- software.amazon.awssdk 2.25.60 2.25.63
- com.google.apis	google-api-services-drive v3-rev20240327-2.0.0	 v3-rev20240521-2.0.0
- org.neo4j.driver	neo4j-java-driver 5.20.0 5.21.0
- org.springframework.integration	spring-integration-mail 6.2.4 6.2.5

Signed-off-by: Joseph Witt <joewitt@apache.org>
Signed-off-by: Pierre Villard <pierre.villard.fr@gmail.com>

This closes apache#8907.

Signed-off-by: Pierre Villard <pierre.villard.fr@gmail.com>

[NIFI-13325] update dark mode theme density to match light mode (apache#8904)

* [NIFI-13325] update dark mode theme density to match light mode

* remove density from nifi themes as only colors are used from this theme

This closes apache#8904

NIFI-12801 Add local file upload option in PutHDFS processor

This closes apache#8415.

Signed-off-by: Peter Turcsanyi <turcsanyi@apache.org>

NIFI-13329 - Updating the standard content viewer to render an error message when there is an error formatting.

Co-authored-by: Pierre Villard <pierre.villard.fr@gmail.com>
Signed-off-by: Pierre Villard <pierre.villard.fr@gmail.com>

This closes apache#8905.

NIFI-13342 restored sts dependency in aws service api

Signed-off-by: Matt Burgess <mattyb149@apache.org>

This closes apache#8910

NIFI-11078: Adds Component UUID to Flow Configuration History Table (apache#8909)

This closes apache#8909

Update BinFiles not to write attributes to FlowFiles for auto-terminated ORIGINAL relationship

Signed-off-by: Matt Burgess <mattyb149@apache.org>

This closes apache#8911

NIFI-13350: (apache#8912)

- Allowing parameters to be edited in New Parameter Context dialog.
- Ensuring the proper tab is selected in the Parameter Context dialog based on the current usage.

This closes apache#8912

NIFI-13138 Add Bundle extensions name and description in NiFi Registry

This closes apache#8740

Signed-off-by: David Handermann <exceptionfactory@apache.org>

NIFI-13352 Adjusted Shutdown handling in ListenOTLP and Test Class
This closes apache#8913

- Added quick duration for shutdown quiet period in ListenOTLP HttpServerFactory
- Added TestRunner.stop() to ListenOTLPTest to close listening sockets
- Increased Connect Timeout from 5 to 10 seconds in ListenOTLPTest

Signed-off-by: Joseph Witt <joewitt@apache.org>

NIFI-13337: (apache#8915)

- Adjust column widths of the queue listing table.

NIFI-13310 merged RAT declarations

Signed-off-by: Pierre Villard <pierre.villard.fr@gmail.com>

This closes apache#8916.

NIFI-13231 Added App Private Key Auth to GitHub FlowRegistryClient

This closes apache#8890

Signed-off-by: David Handermann <exceptionfactory@apache.org>
Co-authored-by: David Handermann <exceptionfactory@apache.org>

[NIFI-13355] move view cluster details and view flow configuration details into action kebab menus (apache#8921)

This closes apache#8921

NIFI-13288 Improved SplitXml and SplitAvro to call session.putAttributes()

This closes apache#8917

Signed-off-by: David Handermann <exceptionfactory@apache.org>

NIFI-13351 Improved QueryDatabaseTable Processors to call session.putAttributes()

This closes apache#8919

Signed-off-by: David Handermann <exceptionfactory@apache.org>

NIFI-13339 Set sensitive as true in USM Users JSON content on ListenTrapSNMP

This closes apache#8908

Signed-off-by: David Handermann <exceptionfactory@apache.org>

[NIFI-13353] improve anchor tag hover state styles in dark mode (apache#8920)

This closes apache#8920

NIFI-13357 Removed APP_INSTALLATION_TOKEN from GitHubFlowRegistryClient (apache#8923)

Signed-off-by: David Handermann <exceptionfactory@apache.org>

[NIFI-13349] align angular material and tailwind typography (apache#8918)

* [NIFI-13349] align angular material and tailwind typography

* override default tailwind fontSize configurations to match up with angular material typography configuration

* cleanup duplicate style

* add text-3xl tailwind configuration

* update primary-node-only to use text-sm

* replace .refresh-container with text-sm

* add comments for $subtitle-2 material typography config

* adjust $subtitle-2 font size and line height

This closes apache#8918

[NIFI-13331] set default table density to -4 for all listings in nifi (apache#8925)

This closes apache#8925

[NIFI-13361] determine extension description height base on $body-2 line-height configuration (apache#8927)

This closes apache#8927

[NIFI-13360] rename nifi theme to supplemental theme (apache#8926)

* [NIFI-13360] rename nifi theme to supplemental theme

* Update nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-frontend/src/main/nifi/apps/nifi/src/assets/styles/_app.scss

Co-authored-by: Rob Fellows <rob.fellows@gmail.com>

* Update nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-frontend/src/main/nifi/apps/nifi/src/assets/styles/_app.scss

Co-authored-by: Rob Fellows <rob.fellows@gmail.com>

---------

Co-authored-by: Rob Fellows <rob.fellows@gmail.com>

This closes apache#8926

NIFI-13030 Adding endpoint for comparing versions of registered flows

This closes apache#8670

Signed-off-by: Peter Gyori <pgyori@apache.org>

NIFI-13313: Remove old UI (apache#8906)

* NIFI-13313:
- Use nifi-web-frontend as the default UI hosted at /nifi no longer deploying nifi-web-ui.

* NIFI-13313:
- Adding logout complete page.
- Updating backend to redirect to new logout complete page.

* NIFI-13313:
- Remove nifi-web-ui module.

* NIFI-13313:
- Updating LICENSE and NOTICE files for dependencies that are no longer included.

* NIFI-13313:
- Updating README.
- Updating proxy config to mirror actual context path.

* NIFI-13313:
- Establishing rewrite rules for redirecting logout complete.
- Setting the default handler for when a request isn't handled to redirect the user to /nifi.

* NIFI-13313:
- Removing nifi-web-error module.

* NIFI-13313:
- Restoring /nifi/logout-complete path.

* NIFI-13313:
- Adding an error handler for the ui which handles redirects to a 404 page.

This closes apache#8906

NIFI-13365 - Fix unit tests running in kubernetes pod

NIFI-13364 Removed autorefresh and banner UI properties

This closes apache#8932

Signed-off-by: David Handermann <exceptionfactory@apache.org>

NIFI-13367: (apache#8933)

- Updating the page title to align with the root Process Group.

This closes apache#8933

NIFI-13368: (apache#8931)

- Allowing tooltip mouse listeners to be destroyed when necessary.
- Ensuring connection source/destination run status and validation errors are updated when deleted.

This closes apache#8931

Add dynamic topic change capability

[NIFI-13370] - dark-mode support for browser inputs (apache#8935)

This closes apache#8935

NIFI-13354: (apache#8936)

- Moving NiFi front end source into a top level maven module. This prepares for the introduction of other UIs that NiFi offers (like Custom UIs, Data Viewers, etc) to be colocated and share common components, styles, and dependencies.
- The nifi-web-frontend module which produces the war that is included in the server nar now no longer contains any source. It simply depends on the new nifi-frontend artifact that bundles all built UIs and unpacks its contents into the resulting war.
- Renaming nifi-web-frontend to nifi-ui. Now nifi-frontend at the top level will hold all frontend apps. In this commit the nifi app in nifi-frontend is bundled into a war called nifi-ui.

This closes apache#8936

NIFI-12983 Qdrant vector store support

Co-authored-by: Pierre Villard <pierre.villard.fr@gmail.com>
Signed-off-by: Pierre Villard <pierre.villard.fr@gmail.com>

This closes apache#8590.

NIFI-13375 Added missing logging parameter in SplitRecord

Signed-off-by: Mike Moser <mosermw@apache.org>

This closes apache#8941

NIFI-13378: Adds Version State as filterable option in Process Groups tab in Summary (apache#8945)

* NIFI-13378: Adds Version State as filterable option in Process Groups tab in Summary

* updated value of selections to enum values

This closes apache#8945

[NIFI-13371] update canvas context menu (apache#8949)

This closes apache#8949

NIFI-13373: Adding support for banner text (apache#8947)

* NIFI-13373:
- Adding support for banner text.

* NIFI-13373:
- Prettier.

* NIFI-13373:
- Removing unused property.

* NIFI-13373:
- Defining reponse payload when loading banner text.
- Removing banner text from login, logout, and error pages.

* NIFI-13373:
- Only loading the banner text when necessary.

This closes apache#8947

NIFI-13385: (apache#8953)

- Disabling nx daemon during builds.

This closes apache#8953

NIFI-13359 Tune ExecuteSQL/Record to create fewer transient flow files

Signed-off-by: Matt Burgess <mattyb149@apache.org>

This closes apache#8928

NIFI-13383 Changed info log level to debug in XXEValidator (apache#8952)

Signed-off-by: David Handermann <exceptionfactory@apache.org>

NIFI-13266 Removed String concatenation in logging messages (apache#8940)

Signed-off-by: David Handermann <exceptionfactory@apache.org>

NIFI-13242 MiNiFi Sync Resource C2 command

Signed-off-by: Ferenc Erdei <erdei.ferenc90@gmail.com>
This closes apache#8898.

NIFI-13391: (apache#8960)

- Setting up different environment files to configure ngrx store.

NIFI-13379 Replaced use of deprecated com.nimbusds.oauth2.sdk.http.HTTPResponse method getContentAsJSONObject with API suggested replacement getBodyAsJSONObject.

This closes apache#8944

Signed-off-by: Mike Thomsen <mthomsen@apache.org>

Improving Junit test to better cover the feature

Remove unnecessary code
Copy link
Contributor

@exceptionfactory exceptionfactory left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for consolidating the commits @andrealves23.

On closer review, I think it is best to avoid introducing this substantive change to the ConsumeKafka_2_6 Processor.

The general purpose of Consume Processors is to continuously receive data from a given source. Supporting input FlowFiles represents a significant change in behavior, beyond simply making the topic name configurable from an input FlowFile attribute.

Not consuming messages from a particular topic for an extended period of time could result in unexpected processing surges. That is not to say this couldn't work, but it introduces a new layer of complexity. If the project were to consider supporting this type of capability, I think a better approach would be to consider a dedicated Processor. Work on NIFI-11259 introduces a redesigned an modernized approach to Kafka integration, which would provide a much better foundation for anything new. There are still a few things to work out, but it is nearing completion. In light of those forthcoming changes, I recommend revisiting this use case after the new components are merged. That would also provide the opportunity to describe potential use cases in more detail on this Jira issue.

@andrealves23
Copy link
Contributor Author

Thanks for closer review @exceptionfactory,

The ConsumeKafka_2_6 processor maintains its original behavior even with flowfile support. It can collect Kafka messages regardless of its connection status and without needing stimulation from a flowfile. The key difference is its new capability to support flowfile processing, allowing for dynamic topic changes. I think this change does not increase complexity significantly; it simply adds a new feature while preserving the original behavior.

@joewitt
Copy link
Contributor

joewitt commented Jun 21, 2024

@andrealves23 First thank you for contributing. I can appreciate the intent. That said I do agree with the response of @exceptionfactory.

A couple things at play. First is related to the maintainability of the Kafka components. We should be getting rid of the Kafka 2.x components soon in favor of Kafka 3.7/latest or even 4.0 as it will be out very soon. There is a long standing active effort to refactor the Kafka processors underway as well. And of course this is just consume kafka and we also have record oriented processors as well. We can't have behavior deviating too far.

Secondarily there is then the question of this change relative to the purpose of the processor. That processor is written as a source component and designed as such. Switching to allow it to accept flowfiles as input which are effectively signals to change which topic(s) it pays attention to is likely not producing enough of a clear user experience improvement.

As I read this it means the input flowfiles act as signals/switches to change which topic is being monitored. In your intent/flow how will that really work in terms of 'what' creates the input flowfiles and at what rate will new flowfiles arrive? What happens if a new flowfile comes in and there is still more to read on the topic(s) in question/etc.?

This is too complex as-is for a user and changes an otherwise extremely critical/common use component.

To be clear i'm not rejecting the idea. You may well be on to something. I suggest it is offered with a more detailed writeup of the 'why' and how it would work in practice/is intended to work. Then a component that is specific to this DynamicConsumeKafka* pattern. This way we can all offer thoughts on the first part before having to react to the code.

The writeup on the JIRA is a good start but what it lacks is a discussion/description around 'what and how' signaling of topic changes would occur. We need to understand more of that intent/picture.

@exceptionfactory
Copy link
Contributor

Thanks for the additional perspective @joewitt.

@andrealves23 new Kafka Processors and Controller Services are now part of the main branch following the merge of #8463.

As Joe suggested, I recommend writing up a more detailed use case of how this dynamic topic polling would work in the associated Jira issue. Although the code changes may be small, the implications for flow design are very significant. If you can describe the general concept there, and what should drive selecting a topic to poll, that would be very helpful.

With the new KafkaConnectionService API, there is now an abstraction around producing or consuming Kafka records, avoiding tight coupling between NiFi Processors and the Kafka client library version. This new Controller Service API also provides a new way to introduce different Kafka-based Processors.

With that background, I am closing this current pull request, but feel free to continue the discussion on the Jira after evaluating the options based on the new components.

@andrealves23
Copy link
Contributor Author

I understand your perspectives, @exceptionfactory and @joewitt.

Therefore, as requested, I added more details to the ticket description to explain why it is essential for Apache NiFi to support dynamic Kafka topic management.

I haven't investigated the new Kafka processor code yet, but the idea is to discuss via the Jira ticket whether this feature can genuinely help and improve the user experience. If it is something we decide to move forward with, I will review the new code and determine the best way to add this feature to NiFi.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants