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

Feature/nifi integration address text #755

Merged

Conversation

QuanticPony
Copy link
Contributor

Proposal to add a property in NiFi-integration processors to allow users to choose address definition strategy and add support for Expression Language in addresses definition.

Issues related:

Actual behavior:

Addresses are specified one by one as dynamic properties in the processor and stored in a map when onScheduled method is triggered.
Expression Language is not supported.

Proposed behavior:

Let user decide between 2 strategies to obtain addresses in the processor:

  • Properties as addresses (as actual behavior but with Expression Language): 1 address per dynamic property.
  • Property "Address Text": a property where all addresses can be specified as field-value pairs in a JSON.

An address map created every time when onTrigger method is called.

@ottobackwards
Copy link
Contributor

ottobackwards commented Jan 20, 2023

Thanks for the contribution!
The issues with expression language for addresses and having multiple addresses in that property were at the time:

  • Putting something complicated to write in a text field is poor for the user
  • The original didn't use json, but used special character substitution changing the native addresses which was a no-go
  • The addresses / returns need to have their PLC types resolved, and if the address could change every call ( which you have to assume if you use expression language ) then it would be good to have some kind of caching so that it wasn't so costly every time, and the original PR did not provide that

In order for this PR to land, it would have to address the caching issue. Having json addresses the "don't invent a custom addressing delimiter" issue. We would just have to live with how poor writing complicated json in that UI would be ( or doing it external and cut and pasting it in)

@QuanticPony
Copy link
Contributor Author

@ottobackwards regarding the cache:
I have added a cache to store addresses-to-PlcTags and addresses-to-RecordSchema mappings.

I have done some tests and seems to speed up ut to 10x the processors.

At the moment each processor has its cache and a new property: Schema Cache Size with default value 1.

Waiting for feedback

* Test addSchema
* Test first in first out schema override in addSchema
* Change lastSchemaPosition to nextSchemaPosition
* Refactor setCacheSize to restartCache
* Change validator for cache size to positive integer (>0)
@QuanticPony
Copy link
Contributor Author

Thank you for your feedback @ottobackwards!
I have made commits addressing all suggested changes

@ottobackwards
Copy link
Contributor

This looks good. If you are ready, I will let you move it from Draft to ready for review, then I'll finish up

@QuanticPony QuanticPony marked this pull request as ready for review February 4, 2023 17:40
@ottobackwards ottobackwards merged commit 0fe3bb5 into apache:develop Feb 7, 2023
@hutcheb
Copy link
Contributor

hutcheb commented Feb 9, 2023

Just going through and removing jackson being pinned at version 2.14.1, this will result in it being bumped to 2.14.2.

This was causing an issue with the build. Let me know if there was a specific reason this was being held back.

@QuanticPony
Copy link
Contributor Author

Must be a left over from testing. There is not any reason, sorry for the inconvenience

@QuanticPony QuanticPony deleted the feature/nifi-integration-address-text branch March 1, 2023 12:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants