opensearch-project / common-utils

Offers a library of utilities for building Java-based OpenSearch plugins
Apache License 2.0
20 stars 93 forks source link

Merge Common Utils into opensearch-project/OpenSearch #114

Open CEHENKLE opened 2 years ago

CEHENKLE commented 2 years ago

Is your feature request related to a problem? Please describe. Common Utils was stood up as an independent repo back in the days of ODFE. We have not revisited whether this still makes sense in an OpenSearch world :)

This issue is to evaluate if it makes sense to move common utils into core, and if so how/where.

Please note: Common Utils right now is a grab bag of different features and functions, many of which are marked deprecated. We'd want to make sure we were moving over only the things that made sense in a mindful manner, rather than a straight up "lift and shift" as is.

Require work: Accounting and evaluation of what's currently in common utils. For each item, determining the best location to move it into core (or into another plug in if applicable).

peternied commented 2 years ago

Common Utils has presented with ownership hiccups with everyone and no-one being responsible directly for it. I would prefer the clarity of putting it inside the more actively managed codebase

nknize commented 2 years ago

We have not revisited whether this still makes sense in an OpenSearch world :)

Seems premature. I think this :point_up: should be resolved before any refactoring to core is done?

dblock commented 2 years ago

We have not revisited whether this still makes sense in an OpenSearch world :)

Seems premature. I think this ☝️ should be resolved before any refactoring to core is done?

It seems like merging it into core is a way to assign ownership. This may or may not be bad.

@nknize Do you have technical arguments not to?

peternied commented 2 years ago

We have not revisited whether this still makes sense in an OpenSearch world :)

CommonUtils was a mechanism for all proto-OpenSearch codebases to shared code. In the era of OpenSearch, the utility of this code as a separate library has been minimized.

If someone proposed making a CommonUtils2.0 repository today I would vote against its creation.

nknize commented 2 years ago

@nknize Do you have technical arguments not to?

Posted at https://github.com/opensearch-project/OpenSearch/issues/2545#issuecomment-1076378627

Repost

:-1: for 2.0.0 it won't be ready:

  1. There is a lot of legacy code that should be removed before refactoring to core
  2. There is kotlin code that we need to tie in w/ the build hooks
  3. The Integration Tests are Disabled w/ a note "Enable this after integration with security plugin is done" So why aren't we just integrating the appropriate classes with the security plugin?
  4. There is no bwc integration tests and that will take some time to work out in core.

:+1: for a future release if targeting core makes sense. We might want to integrate with security first (or dissolve the implementations across other plugins) then integrate security with core rather than blindly integrate with core.

CEHENKLE commented 2 years ago

@praveensameneni Can you share on this issue (or point us to another issue) your analysis of the state of common utils and your plan to clean up the issues @nknize pointed out?

Thanks!

CEHENKLE commented 2 years ago

@praveensameneni Hi, I'd like to pick this back up and dust it off. I think folding common utils into core is the right direction, but I don't want to do that until the items Nick mentioned are done. Are you actively working on that, and if so, what's the ETA?

praveensameneni commented 2 years ago

@CEHENKLE , Just some background, we created common-utils for supporting fine grained access in different plugins which has been adopted by many plugins - alerting, Anomaly Detection, Index Management, Async search among others. Over time few other plugins added code into common-utils. In addition to the items that @nknize posted, it needs to be re-designed and this will impact several plugins. We are not actively working on the re-design of common-utils.

dblock commented 2 years ago

I must point out that this existential issue was written on February 11th, which is the 42nd day of the year.

raj-chak commented 1 year ago