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

Standard for Split/Renamed Repos #3005

Closed
derek-ho opened this issue Dec 14, 2022 · 20 comments
Closed

Standard for Split/Renamed Repos #3005

derek-ho opened this issue Dec 14, 2022 · 20 comments

Comments

@derek-ho
Copy link
Contributor

derek-ho commented Dec 14, 2022

Issue:
Recently, observability repo was renamed to dashboards-observability, and dashboards-reports was renamed to dashboards-reporting. To reflect this, the manifests for 2.4.1 and 1.3.7 were updated with the new repo names. Going forward, the backend folders of these repos will be moved to observability and reporting, respectively. This causes some confusion in terms of what to do with historical manifests for OS and OSD since they will not be accurate anymore and may need to be changed if artifacts need to be rebuilt.

Let us come to a decision and document it for any other similar situations going forward

Ask:

  • Are there any other changes that need to be made from the build-repo side to support this name change and repo splitting/creation?
@derek-ho derek-ho changed the title Standard for Split Repos Standard for Split/Renamed Repos Dec 14, 2022
@derek-ho
Copy link
Contributor Author

@CEHENKLE @dblock @peterzhuamazon @gaiksaya Please add any thoughts you have so we can set the standard and continue with the repo splitting/creation process, thanks! CC @vamsi-amazon

@derek-ho
Copy link
Contributor Author

The same applies to sql repo, which will now be split into dashboards-query-workbench, sql-cli, sql-odbc, and sql-jdbc. @YANG-DB @sejli

@dblock
Copy link
Member

dblock commented Dec 14, 2022

What confusion or actual problem does this change cause? GitHub continues to redirect to old names and all git references remain stable. AFAIK no changes in previous manifests are required and everything continues to work.

@derek-ho
Copy link
Contributor Author

derek-ho commented Dec 14, 2022

@dblock here's an example to make the problem more clear in the 2.4.1 opensearch manifest file there is the following entry for opensearch-observability:

    repository: https://github.com/opensearch-project/dashboards-observability.git
    ref: tags/2.4.1.0
    platforms:
      - linux
      - windows
    working_directory: opensearch-observability
    checks:
      - gradle:properties:version
      - gradle:dependencies:opensearch.version

Once the new repo observability is created and the backend folder is moved there this entry will no longer be the correct location to get the plugin. The same issue exists for the front end plugin in manifests older than 1.3.7 and 2.4.1. Additionally we are looking into preserving git history in the new repo, but don't yet have a guarantee that we will have SHA/branching preservation in the new repo, so any references (I saw a few in the repo, but not sure if they are relevant) or dependencies may be broken CC @pjfitzgibbons

@pjfitzgibbons
Copy link

Yep. 👀 There are multiple workflows we'll have to iron out as soon as we get these repos online. We'll also have to copy/paste several workflows into the "new" repo to make builds run right. Infra/Build teams can help us with those, as well.

@pjfitzgibbons
Copy link

As far as history, I am leaning toward simply making a "cleanup" commit that excises the "unused" side of the codebase for each repo... past history will be duplicate in each side, some well-known tag will identify when we split.

I am open to thoughts and other approaches to this.

@dblock
Copy link
Member

dblock commented Dec 15, 2022

@dblock here's an example to make the problem more clear in the 2.4.1 opensearch manifest file there is the following entry for opensearch-observability:

    repository: https://github.com/opensearch-project/dashboards-observability.git
    ref: tags/2.4.1.0
    platforms:
      - linux
      - windows
    working_directory: opensearch-observability
    checks:
      - gradle:properties:version
      - gradle:dependencies:opensearch.version

Once the new repo observability is created and the backend folder is moved there this entry will no longer be the correct location to get the plugin.

I am confused what got renamed to what. Above you say "observability repo was renamed to dashboards-observability". So what is that we have in the manifest and what did we rename what to?

If you rename a repo, it has no effect on the manifest.

If you move a folder, you don't move it on an existing tag, you move it in another branch (e.g. main). So nothing should change there either.

@gaiksaya
Copy link
Member

I think some information is missing from this issue. The issue will be in case of renaming and creating a new repo with old name.
For example, I believe the plan is to rename the current dashboards-reporting repo to reporting (backend plugin)
and then create a new repo called dashboards-reporting for dashboards plugin.
What should be the approach here @dblock ?

@dblock
Copy link
Member

dblock commented Dec 19, 2022

@gaiksaya I think we shouldn't do this and create a new repo called reporting.

@derek-ho
Copy link
Contributor Author

derek-ho commented Dec 19, 2022

@dblock yea reporting should be ok, but for observability we don't really have a choice, since we want to follow opensearch org naming convention of not including opensearch in the name. Thus, the only two feasible names are observability and dashboards-observability. Thus the conflict with no redirect available.... Regardless of if there is a redirect available or not, one half of the code base will be found at a different repo location, causing an issue with manifest accuracy.

@dblock
Copy link
Member

dblock commented Dec 20, 2022

@derek-ho I don't think I understand. The manifests for versions that have shipped are locked a revision. In that revision the code will not have moved yet. Nothing will break.

@derek-ho
Copy link
Contributor Author

@dblock ok... I am not familiar with how manifest versions are locked on revisions. I just wanted to make sure we don't break anything from the build repo side. For example for 2.4.0, the build repo would be looking at https://github.com/opensearch-project/dashboards-observability.git under the folder opensearch-observability for the backend plugin, but once the new repo is created and redirect breaks, this would ideally be located at https://github.com/opensearch-project/observability.git. In case we need to rebuild anything I just wanted to make sure nothing will break or fail.

@dblock
Copy link
Member

dblock commented Dec 21, 2022

Your example is incorrect I think - 2.4.0 has a reference to observability here: https://github.com/opensearch-project/opensearch-build/blob/main/manifests/2.4.0/opensearch-dashboards-2.4.0.yml#L20

The original plan was to rename observability to dashboards-observability and create observability, which would create exactly the problem that you describe. Thus, it was a bad idea. Instead we left https://github.com/opensearch-project/observability alone, and https://github.com/opensearch-project/dashboards-observability was created (it's currently private waiting for some basic setup/files for a new repo).

Does this make sene? I believe this should be closed.

@derek-ho
Copy link
Contributor Author

@dblock sorry I mistyped - I was talking about 2.4.1 I think this issue still exists for this manifest file since it was changed to dashboards-observability to be safe for the release - https://github.com/opensearch-project/opensearch-build/blob/main/manifests/2.4.1/opensearch-dashboards-2.4.1.yml#L20. We can close it, but just wanted to raise the concern and make sure everyone is on the same page about a path forward.

@dblock
Copy link
Member

dblock commented Dec 21, 2022

Ok, so for 2.4.1 that manifest is wrong - I imagine that happened half way through the rename and now we went back.You should PR a fix for it and change it back to observability like in 2.4.0 because now that https://github.com/opensearch-project/dashboards-observability exists and is empty, that manifest will break.

@pjfitzgibbons
Copy link

pjfitzgibbons commented Dec 21, 2022 via email

@derek-ho
Copy link
Contributor Author

derek-ho commented Jan 5, 2023

@dblock @pjfitzgibbons @gaiksaya @peterzhuamazon @gaiksaya @CEHENKLE @YANG-DB @rupal-bq @anirudha can we make a final decision about this - any concerns? Insights team (observability, sql, reporting) wants to release 2.5.0 with separate repos, and we need confirmation from build/infra team on if this is ok/any changes that need to be made to support this. Example PR to deprecate code from old repo: opensearch-project/observability#1362. New repos will soon be on parity with old ones. What will we be doing with old manifest files?

@dblock
Copy link
Member

dblock commented Jan 5, 2023

I re-read the thread and I am still confused. Old manifest files should continue working because of GitHub redirects and stable references. Do you have an example of a specific manifest file that is now broken with the changes above?

What will we be doing with old manifest files?

If such example doesn't exist I propose to close this issue.

@pjfitzgibbons
Copy link

pjfitzgibbons commented Jan 6, 2023 via email

@bbarani
Copy link
Member

bbarani commented Jan 25, 2023

Closing this issue as we have officially split the monorepos With 2.5.0 release.

@bbarani bbarani closed this as completed Jan 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants