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

SKA: Relocate Script v7 #205732

Merged

Conversation

gsoldevila
Copy link
Contributor

@gsoldevila gsoldevila commented Jan 7, 2025

Summary

Addresses the following:

  • Simplify isInTargetFolder, and leverage existing calculateModuleTargetFolder. It solves a bug with "current" location being incorrectly determined, as we were using the group and visibility inferred from path, rather than those in the manifest.
  • Move the pre-relocation hook to BEFORE calculating the list of modules. This allows to update a manifest to re-relocate a module (e.g. when changing its group or visibility).
  • Fix a bug that caused modules under src/core/packages to not be considered in the "correct location".
  • Fix a bug in the replace logic specific to pipeline.ts. We were updating paths that we shouldn't have updated.
  • Adds a --list all flag to allow listing packages according to their SKA status.

@gsoldevila gsoldevila added Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc release_note:skip Skip the PR/issue when compiling release notes backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) labels Jan 7, 2025
@gsoldevila gsoldevila requested a review from a team as a code owner January 7, 2025 13:34
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-core (Team:Core)

@@ -19,7 +19,6 @@ import {
KIBANA_FOLDER,
NO_GREP,
SCRIPT_ERRORS,
TARGET_FOLDERS,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This constant is stale now and can be safely removed.

Copy link
Contributor

@dokmic dokmic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job! LGTM 👍

@gsoldevila gsoldevila enabled auto-merge (squash) January 7, 2025 13:59
@gsoldevila gsoldevila force-pushed the kbn-team-1066-relocate-script-v7 branch from 38d49a0 to 3e18e9d Compare January 7, 2025 17:20
@gsoldevila gsoldevila merged commit ca42d93 into elastic:main Jan 7, 2025
8 checks passed
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/12657985587

@elasticmachine
Copy link
Contributor

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #111 / Index Management app Index Management: data streams tab Data streams tab Modify data streams index mode allows to downgrade data stream from logsdb to standard index mode

Metrics [docs]

✅ unchanged

@kibanamachine
Copy link
Contributor

💔 All backports failed

Status Branch Result
8.x Backport failed because of merge conflicts

Manual backport

To create the backport manually run:

node scripts/backport --pr 205732

Questions ?

Please refer to the Backport tool documentation

@gsoldevila
Copy link
Contributor Author

💚 All backports created successfully

Status Branch Result
8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

gsoldevila added a commit to gsoldevila/kibana that referenced this pull request Jan 7, 2025
## Summary

Addresses the following:
* Simplify `isInTargetFolder`, and leverage existing
`calculateModuleTargetFolder`. It solves a bug with "current" location
being incorrectly determined, as we were using the `group` and
`visibility` inferred from path, rather than those in the manifest.
* Move the `pre-relocation` hook to BEFORE calculating the list of
modules. This allows to update a manifest to re-relocate a module (e.g.
when changing its group or visibility).
* Fix a bug that caused modules under `src/core/packages` to not be
considered in the "correct location".
* Fix a bug in the replace logic specific to `pipeline.ts`. We were
updating paths that we shouldn't have updated.

(cherry picked from commit ca42d93)

# Conflicts:
#	packages/kbn-relocate/utils/relocate.ts
gsoldevila added a commit that referenced this pull request Jan 7, 2025
# Backport

This will backport the following commits from `main` to `8.x`:
- [SKA: Relocate Script v7
(#205732)](#205732)

<!--- Backport version: 8.9.8 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Gerard
Soldevila","email":"[email protected]"},"sourceCommit":{"committedDate":"2025-01-07T19:27:36Z","message":"SKA:
Relocate Script v7 (#205732)\n\n## Summary\r\n\r\nAddresses the
following:\r\n* Simplify `isInTargetFolder`, and leverage
existing\r\n`calculateModuleTargetFolder`. It solves a bug with
\"current\" location\r\nbeing incorrectly determined, as we were using
the `group` and\r\n`visibility` inferred from path, rather than those in
the manifest.\r\n* Move the `pre-relocation` hook to BEFORE calculating
the list of\r\nmodules. This allows to update a manifest to re-relocate
a module (e.g.\r\nwhen changing its group or visibility).\r\n* Fix a bug
that caused modules under `src/core/packages` to not be\r\nconsidered in
the \"correct location\".\r\n* Fix a bug in the replace logic specific
to `pipeline.ts`. We were\r\nupdating paths that we shouldn't have
updated.","sha":"ca42d93bd46a2e8f46825626f3c45ded80b2da13","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Team:Core","release_note:skip","v9.0.0","backport:prev-minor"],"number":205732,"url":"https://github.com/elastic/kibana/pull/205732","mergeCommit":{"message":"SKA:
Relocate Script v7 (#205732)\n\n## Summary\r\n\r\nAddresses the
following:\r\n* Simplify `isInTargetFolder`, and leverage
existing\r\n`calculateModuleTargetFolder`. It solves a bug with
\"current\" location\r\nbeing incorrectly determined, as we were using
the `group` and\r\n`visibility` inferred from path, rather than those in
the manifest.\r\n* Move the `pre-relocation` hook to BEFORE calculating
the list of\r\nmodules. This allows to update a manifest to re-relocate
a module (e.g.\r\nwhen changing its group or visibility).\r\n* Fix a bug
that caused modules under `src/core/packages` to not be\r\nconsidered in
the \"correct location\".\r\n* Fix a bug in the replace logic specific
to `pipeline.ts`. We were\r\nupdating paths that we shouldn't have
updated.","sha":"ca42d93bd46a2e8f46825626f3c45ded80b2da13"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","labelRegex":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/205732","number":205732,"mergeCommit":{"message":"SKA:
Relocate Script v7 (#205732)\n\n## Summary\r\n\r\nAddresses the
following:\r\n* Simplify `isInTargetFolder`, and leverage
existing\r\n`calculateModuleTargetFolder`. It solves a bug with
\"current\" location\r\nbeing incorrectly determined, as we were using
the `group` and\r\n`visibility` inferred from path, rather than those in
the manifest.\r\n* Move the `pre-relocation` hook to BEFORE calculating
the list of\r\nmodules. This allows to update a manifest to re-relocate
a module (e.g.\r\nwhen changing its group or visibility).\r\n* Fix a bug
that caused modules under `src/core/packages` to not be\r\nconsidered in
the \"correct location\".\r\n* Fix a bug in the replace logic specific
to `pipeline.ts`. We were\r\nupdating paths that we shouldn't have
updated.","sha":"ca42d93bd46a2e8f46825626f3c45ded80b2da13"}}]}]
BACKPORT-->
kowalczyk-krzysztof pushed a commit to kowalczyk-krzysztof/kibana that referenced this pull request Jan 7, 2025
## Summary

Addresses the following:
* Simplify `isInTargetFolder`, and leverage existing
`calculateModuleTargetFolder`. It solves a bug with "current" location
being incorrectly determined, as we were using the `group` and
`visibility` inferred from path, rather than those in the manifest.
* Move the `pre-relocation` hook to BEFORE calculating the list of
modules. This allows to update a manifest to re-relocate a module (e.g.
when changing its group or visibility).
* Fix a bug that caused modules under `src/core/packages` to not be
considered in the "correct location".
* Fix a bug in the replace logic specific to `pipeline.ts`. We were
updating paths that we shouldn't have updated.
crespocarlos pushed a commit to crespocarlos/kibana that referenced this pull request Jan 8, 2025
## Summary

Addresses the following:
* Simplify `isInTargetFolder`, and leverage existing
`calculateModuleTargetFolder`. It solves a bug with "current" location
being incorrectly determined, as we were using the `group` and
`visibility` inferred from path, rather than those in the manifest.
* Move the `pre-relocation` hook to BEFORE calculating the list of
modules. This allows to update a manifest to re-relocate a module (e.g.
when changing its group or visibility).
* Fix a bug that caused modules under `src/core/packages` to not be
considered in the "correct location".
* Fix a bug in the replace logic specific to `pipeline.ts`. We were
updating paths that we shouldn't have updated.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) release_note:skip Skip the PR/issue when compiling release notes Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v8.18.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants