konpyutaika / nifikop

The NiFiKop NiFi Kubernetes operator makes it easy to run Apache NiFi on Kubernetes. Apache NiFI is a free, open-source solution that support powerful and scalable directed graphs of data routing, transformation, and system mediation logic.
https://konpyutaika.github.io/nifikop/
Apache License 2.0
122 stars 40 forks source link

Update nifi.properties template for NiFi 2.0.0-M1 #344

Open juldrixx opened 6 months ago

juldrixx commented 6 months ago
Q A
Bug fix? no
New feature? no
API breaks? no
Deprecations? ?
Related tickets partial #360
License Apache 2.0

What's in this PR?

Update of the nifi.properties template.

Why?

To add the new properties and the changes on the old ones for 2.0.0-M1.

Checklist

r65535 commented 6 months ago

Is it worth looking at replacing the nifi.properties logic we have here, to make use of the ENV scripts in apache/nifi's docker image instead? My worry with using templates is that we have a single, local copy of nifi.properties covering multiple minor/major versions of NiFi. I've skimmed through the difference between this PR's template and 2.0.0-M1 and there are dozens of differences (properties being removed, defaults being different etc). If we take nifi.properties values in the CRD and convert them into pod ENVs instead, we don't need a local template - and this should work across multiple versions?

mh013370 commented 6 months ago

Is it worth looking at replacing the nifi.properties logic we have here, to make use of the ENV scripts in apache/nifi's docker image instead? My worry with using templates is that we have a single, local copy of nifi.properties covering multiple minor/major versions of NiFi. I've skimmed through the difference between this PR's template and 2.0.0-M1 and there are dozens of differences (properties being removed, defaults being different etc). If we take nifi.properties values in the CRD and convert them into pod ENVs instead, we don't need a local template - and this should work across multiple versions?

Hmm i think we ought to consider doing this as well. And NiFi 2.0.0 would be a good time to introduce breaking changes in nifikop as well, if we find that it's needed.

One minor hurdle may be that NiFi doesn't currently support being entirely configured through environment variables. I think the majority of properties are covered, but it's not a general solution as it's currently written. I saw a PR last week to add support for more properties. For that reason, we may need to keep both options around.

juldrixx commented 6 months ago

Is it worth looking at replacing the nifi.properties logic we have here, to make use of the ENV scripts in apache/nifi's docker image instead? My worry with using templates is that we have a single, local copy of nifi.properties covering multiple minor/major versions of NiFi. I've skimmed through the difference between this PR's template and 2.0.0-M1 and there are dozens of differences (properties being removed, defaults being different etc). If we take nifi.properties values in the CRD and convert them into pod ENVs instead, we don't need a local template - and this should work across multiple versions?

It's why I didn't removed them, just added the new ones. To stay compatible with previous version.

juldrixx commented 6 months ago

Is it worth looking at replacing the nifi.properties logic we have here, to make use of the ENV scripts in apache/nifi's docker image instead? My worry with using templates is that we have a single, local copy of nifi.properties covering multiple minor/major versions of NiFi. I've skimmed through the difference between this PR's template and 2.0.0-M1 and there are dozens of differences (properties being removed, defaults being different etc). If we take nifi.properties values in the CRD and convert them into pod ENVs instead, we don't need a local template - and this should work across multiple versions?

Hmm i think we ought to consider doing this as well. And NiFi 2.0.0 would be a good time to introduce breaking changes in nifikop as well, if we find that it's needed.

One minor hurdle may be that NiFi doesn't currently support being entirely configured through environment variables. I think the majority of properties are covered, but it's not a general solution as it's currently written. I saw a PR last week to add support for more properties. For that reason, we may need to keep both options around.

And I'm not sure, it will work with custom image.