pvillard31 commented on PR #10186: URL: https://github.com/apache/nifi/pull/10186#issuecomment-3179595270
Thanks @exceptionfactory > I am concerned about adding the Cache Directory property, because it introduces potential compatibility concerns on future upgrades. Although the cache format may not change very often, it introduces an additional layer of compatibility concern down the road. It seems better to restrict caching to something memory-based. I did think about adding a custom in-memory cache via an OkHttp interceptor but I'm concerned about the size in case the flows that we retrieve are particularly huge. Unless I limit the interceptor to very specific endpoints being called like list commits, get commit details. But it felt overcomplicated. To be honest the most significant improvements are the commits caching and limiting the number of commits being retrieved so I'm also OK dropping the OkHttp caching for now. > As far as maximum commits, having a limit seems reasonable, but I'm also not sure if this needs to be a configurable property. Setting a reasonable maximum seems like a safety feature to avoid unnecessary consumption, and going beyond a certain limit seems unnecessary and potentially unusable in the context of reviewing changes. Perhaps setting an internal limit of 1000 is the best approach for now? I did think about it and I don't have a strong opinion. Something that I did consider is to add a property "Enable Caching" true/false that would make visible all of the new properties (including the one about how many commits are retrieved?) and expose this "magic number" that is currently set to 1000. No strong opinion tbh. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
