Dennis-Mircea commented on PR #1085: URL: https://github.com/apache/flink-kubernetes-operator/pull/1085#issuecomment-4587091678
Raising the same comment as in #1099. Coming back to this after stepping back to look at the broader module structure. Wanted to raise a design question before merge, because it affects what custom-plugin authors will compile against. This applies to #1099 PR as well. ### The asymmetry today The operator's plugin SPIs are split across two modules: | Module | SPIs it carries | |---|---| | `flink-kubernetes-operator-api` (thin) | `FlinkResourceListener` | | `flink-kubernetes-operator` (implementation, heavy) | `FlinkResourceValidator`, `FlinkResourceMutator` | A custom-validator or custom-mutator author has to depend on the heavy implementation module (which transitively pulls JOSDK, fabric8, the autoscaler, etc.) just to compile against one interface. A custom-listener author depends on a much thinner `-api` artifact. The asymmetry is historical, not designed. ### The question This PR adds new plugin-style SPIs to `flink-autoscaler`, which is the implementation module. Would it be cleaner to land them in a dedicated `flink-autoscaler-api` module instead, so the user-facing answer to "what do I import to write an autoscaler plugin?" is a single thin artifact? The plugin author's first question when implementing a new SPI is always "which module do I import?". Today that answer is inconsistent across SPIs and points at heavy implementation artifacts in most cases. Landing these new SPIs in a thin `flink-autoscaler-api` would set a cleaner precedent. ### Why now, not after merge Once the new SPIs are in `flink-autoscaler`, the project implicitly commits to "autoscaler plugin SPIs live in the implementation module" as the pattern. A later restructuring becomes a multi-PR migration that breaks plugin-author coordinates a second time. The cost of asking the question now is a slight delay on this PR. The cost of asking it later is significantly higher. ### The broader implication If we want to standardize on "plugin SPIs live in `-api` modules" going forward, that also implies: - Moving `FlinkResourceValidator` and `FlinkResourceMutator` into `flink-kubernetes-operator-api` to restore symmetry with `FlinkResourceListener`. - Creating `flink-autoscaler-api` carrying both the existing `JobAutoScaler` / `ScalingRealizer` / `AutoScalerStateStore` and the new SPIs from this PR. That's a project-wide structural decision and would warrant a `[DISCUSS]` thread on [email protected] before all of it lands. I do not want to block this PR with that broader conversation, but the immediate question (where the new SPIs in this PR should live) shapes the email and is worth aligning on first. @gyfora, what do you think? -- 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]
