Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Enhancement][Multi-Repository Workflow] Better Multi-Repo workflow needed #114

Open
nknize opened this issue Mar 14, 2023 · 12 comments
Open
Assignees
Labels
enhancement New feature or request

Comments

@nknize
Copy link

nknize commented Mar 14, 2023

Is your feature request related to a problem? Please describe.
Changes to opensearch core that break downstream repositories should be caught and repaired earlier.

This comes out of opensearch-project/OpenSearch#6470 (comment)

Describe the solution you'd like
Something like Android's git/repo tools where a checkout of a project / plugin repository can also checkout core opensearch and a meta build.gradle sets up the dependencies to use local source instead of SNAPSHOT builds. This would enable us to make the appropriate change (e.g., refactor) in the downstream project repo and open the PR earlier rather than waiting for cascading build failures across the ecosystem.

Describe alternatives you've considered
Go back to a monolithic repository.

Additional context
Mono vs Multi repository design is not a new concept. I encourage us to spend some time researching prior art, e.g.: https://kinsta.com/blog/monorepo-vs-multi-repo/ to see what's a best fit for the OpenSearch ecosystem.

@nknize nknize added enhancement New feature or request untriaged Issues that have not yet been triaged labels Mar 14, 2023
@gaiksaya gaiksaya removed the untriaged Issues that have not yet been triaged label Mar 14, 2023
@gaiksaya
Copy link
Member

[Triage] Needs discussion and proposal to move forward.

@gaiksaya
Copy link
Member

gaiksaya commented May 16, 2023

Hey @nknize ,

We have been brainstorming about this a bit. Adding thoughts here.

There are multiple changes that can be categorized as refactoring such as package level changes (example Xcontent) or class renamings, etc. It is difficult to have one solution for all in detecting what changes were made in core and what affects the downstream projects.

However, we can start with checking the compatibility of each child project. Similar to manifest checks that we have in build repository, we can have some checks in downstream projects to see if the upstream changes are breaking anything. If we need to run full blown gradle build on each project or can we have a lightweight tasks is yet to be decided after some more research but the gist of it is having an optional gradle task in OpenSearch repo (say ./gradlew check-compatibility) which takes an input of a repo name or multiple repos, checks it out and runs the compatibility check in child projects.

If indeed it is a breaking change for child projects, running ./gradlew check-compatibility --notify will just go ahead and create a GitHub issue in that repo as a heads up about any incoming breaking change.

If the PR author wishes to go ahead and fix the issue for each one of the failing components, its their call but we can start with the notification and then iterate upon it. This can be easily automated from developer manually running the gradle task to GHA doing it and notifying author + creating issues in affecting downstream projects.

For something like package refactoring as mentioned above it is simple string replacement in all repos (for most cases) but as I said that is just one example and can be solved like this https://github.com/opensearch-project/opensearch-build-libraries/blob/main/build.gradle#L125-L132 but for more complicated refactoring we might need a robust solution.

image

Let me know what do you think.
Also adding @dblock @CEHENKLE @bbarani @prudhvigodithi to the conversation for more inputs.

@gaiksaya
Copy link
Member

Looks like ./gradlew assemble is the right task to run from plugins point of view as it is light weight compared to ./gradlew build. Tried below:

  1. Publish opensearch to maven local with xcontent commit
  2. Run ./gradlew assemble on few plugins (alerting, index-management, k-NN, geospatial to be precise) before this change was accommodated in plugins repo and all failed within 15 seconds.

@ylwu-amzn
Copy link

Currently plugin teams develop features based on main branch, which depends on OpenSearch core main. That means if OpenSearch core introduce any breaking change, then plugin team have to pause their feature development work to fix the breaking changes first. And some plugins/component like ml-commons/common-utils consumed by other plugins, that mean other plugin teams need to wait for ml-commons/common-utils team fixing the breaking changes first. That time could be even longer.

Another thing is plugin team never knows there are breaking changes until they publish a PR to main branch.

Is it ok for plugin teams to develop feature on a stable branch like 2.x, so they won't be distracted by the breaking changes from OpenSearch core main.

@dblock
Copy link
Member

dblock commented May 19, 2023

Is it ok for plugin teams to develop feature on a stable branch like 2.x, so they won't be distracted by the breaking changes from OpenSearch core main.

I wouldn't. You're just moving the integration pain to later.

@prudhvigodithi
Copy link
Member

Adding my thoughts.

Firstly I would not consider mono repo with challenges related to the increased complexity of managing a large codebase, longer build times, and potential conflicts among teams working on different projects. This also stops us from idea of releasing individual plugins with their own versions.

Thanks for the thought @gaiksaya.

The ./gradlew check-compatibility can be called during a PR creation (similar to existing gradle check) and add a comment to PR (something like this), that shows the list of plugins that could break with the PR from Core that introduce the breaking changes.

However for the gradle check-compatibility to run might take long time and not recommended to run for every PR, consider executing it with only a breaking-change label added to the PR.

We can start by notifying the plugins early and add intelligence down the line to raise a PR for the plugin that can fix the breaking change.

@ylwu-amzn concern can be solved by locking in the core snapshot artifact from the component repo and then come back to the latest snapshot artifact that had the breaking change, this way the component development would not be halted.

dependencies {
    implementation 'your.group:your.artifact:1.0.0-20230513-120000-XYZ' // Replace with the appropriate timestamp or version
}

@ylwu-amzn
Copy link

ylwu-amzn commented May 22, 2023

@ylwu-amzn concern can be solved by locking in the core snapshot artifact from the component repo and then come back to the latest snapshot artifact that had the breaking change, this way the component development would not be halted.

Thanks @prudhvigodithi , this looks a way to mitigate the pain. Need to coordinate all plugin teams to use the same snapshot, otherwise plugin will have conflicts.

For this one implementation 'your.group:your.artifact:1.0.0-20230513-120000-XYZ' // Replace with the appropriate timestamp or version, Can we avoid using the timestamp? If we can always use your.group:your.artifact:1.0.0-latest, that can save effort for all plugin teams to coordinate versions and bumping snapshot versions by changing the timestamps.

One concern is, if we use the latest compatible snapshot (or no new breaking change snapshot), we may need some way to detect and notify plugin teams that some new breaking changes happens in core. Once plugin teams address the new breaking changes , we need some way to bump the latest OpenSearch core snapshot to new commit.

@gaiksaya
Copy link
Member

gaiksaya commented May 25, 2023

Can we avoid using the timestamp? If we can always use your.group:your.artifact:1.0.0-latest, that can save effort for all plugin teams to coordinate versions and bumping snapshot versions by changing the timestamps

Not specifying anything is latest which is updated per push o core repo.

One concern is, if we use the latest compatible snapshot (or no new breaking change snapshot), we may need some way to detect and notify plugin teams that some new breaking changes happens in core. Once plugin teams address the new breaking changes , we need some way to bump the latest OpenSearch core snapshot to new commit.

Snapshots are automatically pushed to sonatype using this workflow https://github.com/opensearch-project/OpenSearch/blob/main/.github/workflows/publish-maven-snapshots.yml once the PR is merged. The scope that this issue concentrates on is detecting what components are affected (if at all) before the change is merged into core and pushed to snapshots repo.

@gaiksaya
Copy link
Member

gaiksaya commented Jun 2, 2023

See POC for above approach here: https://github.com/opensearch-project/OpenSearch/pull/7899/files
Clone the fork and checkout the branch.

git clone https://github.com/gaiksaya/OpenSearch.git
cd OpenSearch && git checkout comp-check

Run below for specific plugins or later to run compatibility check for all plugins:

./gradlew checkCompatibility -PrepositoryUrls=https://github.com/opensearch-project/common-utils.git,https://github.com/opensearch-project/job-scheduler.git

./gradlew checkCompatibility 

@gaiksaya
Copy link
Member

We have opened a GitHub issues in all plugin repositories to ensure they are ready for checking compatibility. See the linked issues above. This is to reduce false positives and negatives checks.
Thanks!

@bbarani
Copy link
Member

bbarani commented Aug 4, 2023

@gaiksaya Can we close this issue?

@gaiksaya
Copy link
Member

gaiksaya commented Aug 4, 2023

Will let @nknize take the call. Notifying the failing/incompatible components is the next step. However, it can be tracked as a separate effort.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: 📦 Backlog
Development

No branches or pull requests

6 participants