[RFC] How helpful is the compatibility checker

[gaiksaya]

Is your feature request related to a problem? Please describe

Few months back we added a compatibility checker workflow to this repository in order to see if a change in OpenSearch could potentially break the downstream components (plugins in this case).
Associated PR: #8486
Associated GH issue: opensearch-project/opensearch-devops#114

We observed that the comments about compatibility status are usually ignored. Example and many more

Before approaching to next steps which was creating issues with all associated incompatible components giving a heads-up that there is an incoming change that could break your build, we wanted to get the feedback from the community members.

Describe the solution you'd like

This was the approach taken to solve the issue: opensearch-project/opensearch-devops#114 (comment)

Related component

Build

Describe alternatives you've considered

If the compatibility checker is not providing any value, we would like to remove it and save resources as well as reduce noise on the PR

Additional context

No response

[gaiksaya]

OpenSearch maintainers as well plugin maintainers we would like to get your input on this issue so as to take the next steps accordingly.
Thanks!

[peternied]

@gaiksaya To me it is not useful - the security plugin recently had a breaking change from OpenSearch come in and check found it [1] it was completely ignored. Lets save the resources to execute the jobs and the reduce the noise on pull requests.

• [1] Add explicit dependency to validatePom and generatePom tasks. #12807 (comment)

[peternied]

[Triage - attendees 1 2 3 4 5]
@gaiksaya Thanks for creating this RFC, looking forward to the result of this discussion.

[stephen-crawford]

I think the checker itself is/was a good idea. But if people are just ignoring it then I agree with @peternied and we should just get rid of it.

[dblock]

In reviewing PRs such as #12967 I find it helpful to know the change is not breaking any plugins. Maybe there's a way to run it on demand by checking a box in the PR description (similar to how dependabot lets you rebase on demand)?

[peternied]

In reviewing PRs such as #12967 I find it helpful to know the change is not breaking any plugins. Maybe there's a way to run it on demand by checking a box in the PR description (similar to how dependabot lets you rebase on demand)?

@dblock On demand/optional workflow execution sounds interesting - what do you think about making a PR to handle this scenario?

I have a less elegant proposal [1] that removes the workflow while leaving the underlying command in place so ./gradlew checkCompatibility can be executed.

• [1] Remove compatibility checker #12971

[andrross]

I don't find the compatibility checker super useful and would be okay with removing it. Basically, I don't really see a path forward to putting strict enforcement on the check, primarily because a compilation failure from a plugin can be caused for reasons unrelated to the change in the PR, in which case we would not want to block commits from going into OpenSearch. Assuming that is true, then the check becomes "best effort, informational only" and given all the other issues related to flaky test failures then this check will almost always be ignored or buried in the noise. I'm okay with removing or changing it to be on-demand.

[gaiksaya]

Tagging few plugin maintainers to get more inputs:
@ylwu-amzn @navneet1v @vamshin @anirudha @owaiskazi19 @prudhvigodithi @bharath-techie

[bbarani]

The main question here is - Is this surfacing the breaking changes in core with decent accuracy? The first step was to add a comment and next step was to notify on Slack / open an automated GitHub issue on respective plugin repo impacted by a change in core for early warning.

[owaiskazi19]

If we could establish a mechanism to notify plugin teams, either through an issue or a slack bot in their respective channels on the OpenSearch Slack, about a breaking change being introduced from the core's side, it would be beneficial. We wouldn't necessarily need to block the core's PR for the same. Instead, we could either tag the author of the breaking change PR to address the issue or revert the change.

[gaiksaya]

@bbarani @owaiskazi19 That was the next step if this goes forward. Even today, nothing is blocked using this workflow. The workflow comments the status on the pull request rather than showing it as failed and blocking the CI. The intention of this issue is to detect whether to move to next step or stop here.
Notification would be helpful agreed, but wondering if this will be adding to the noise of other autocuts like distribution failure or integration test failures. Many of those issues in plugin repos for past releases have not been triaged.

The intention of the compatibility checker was to be more pro-active for incoming breaking change informing both the pull request author as well as downstream components.

[peternied]

@gaiksaya I think we should remove this check. [1] An Java API compatibility checker has been added to the pull request workflow [2], I think this is a better long term solution to the problem.

What do you think about shifting in this direction to remove this check and close out this RFC?

• [1] Remove compatibility checker #12971
• [2] Detect breaking changes on pull requests v2.0 #12974

[gaiksaya]

Thanks @peternied ! I have approved the PR. Looks like it is not providing more value as of now. We can always add it back if required. The main difference that I am seeing with these 2 approaches is that, compatibility checker concentrated only on the components that fell under opensearch distribution See the list. However, the breaking change detector is more universal.

Once the PR is merged, I believe we can close this RFC.