Skip to content

Remove TS reference from stack_connectors to dashboard#208165

Merged
pgayvallet merged 4 commits intoelastic:mainfrom
pgayvallet:kbn-xxx-remove-stack-connector-dash-ref
Jan 28, 2025
Merged

Remove TS reference from stack_connectors to dashboard#208165
pgayvallet merged 4 commits intoelastic:mainfrom
pgayvallet:kbn-xxx-remove-stack-connector-dash-ref

Conversation

@pgayvallet
Copy link
Copy Markdown
Contributor

@pgayvallet pgayvallet commented Jan 24, 2025

Summary

In #159075 was introduced a reference from the stack_connectors plugin to the dashboard plugin. This dependency was made so that the logic creating the GenAI dashboard has proper typing for the dashboard SO attributes type.

However, that is a bad isolation of concerns, introducing a high risk of cyclic dependencies: the dashboard plugin, or any of its dependencies, can't depend on stack_connectors, or any plugin depending on stack_connectors.

This is especially problematic because of another bad design decision of having dashboard depending on observability_ai_assistant, as it increases the risk of circular dependencies.

This PR addresses it by removing the few usages we have of the dashboard plugin's type in stack_connectors. Of course the ideal fix would be to extract the type from the dashboard plugin instead, but those types we're using here are inferred from config schema concrete objects, making it complicated, so I went the pragmatic road even if it means loosing a bit of type safety.

@pgayvallet
Copy link
Copy Markdown
Contributor Author

/ci

@pgayvallet pgayvallet added release_note:skip Skip the PR/issue when compiling release notes v9.0.0 backport:version Backport to applied version labels v8.18.0 labels Jan 24, 2025
@pgayvallet pgayvallet marked this pull request as ready for review January 24, 2025 13:01
@pgayvallet pgayvallet requested a review from a team as a code owner January 24, 2025 13:01
@pgayvallet pgayvallet enabled auto-merge (squash) January 28, 2025 13:43
@pgayvallet
Copy link
Copy Markdown
Contributor Author

@elasticmachine merge upstream

@pgayvallet
Copy link
Copy Markdown
Contributor Author

@elasticmachine merge upstream

@pgayvallet pgayvallet merged commit 5b62617 into elastic:main Jan 28, 2025
@kibanamachine
Copy link
Copy Markdown
Contributor

Starting backport for target branches: 8.x

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

@elasticmachine
Copy link
Copy Markdown
Contributor

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] Jest Tests #7 / useCopyIDAction copies the id of the selected case to the clipboard

Metrics [docs]

✅ unchanged

History

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Jan 28, 2025
…08165)

## Summary

In elastic#159075 was introduced a reference from the `stack_connectors` plugin
to the `dashboard` plugin. This dependency was made so that the logic
creating the GenAI dashboard has proper typing for the dashboard SO
attributes type.

However, that is a bad isolation of concerns, introducing a high risk of
cyclic dependencies: the `dashboard` plugin, or any of its dependencies,
can't depend on `stack_connectors`, or any plugin depending on
stack_connectors.

This is especially problematic because of another bad design decision of
having `dashboard` depending on `observability_ai_assistant`, as it
increases the risk of circular dependencies.

This PR addresses it by removing the few usages we have of the dashboard
plugin's type in stack_connectors. Of course the ideal fix would be to
extract the type from the dashboard plugin instead, but those types
we're using here are inferred from config schema concrete objects,
making it complicated, so I went the pragmatic road even if it means
loosing a bit of type safety.

(cherry picked from commit 5b62617)
@kibanamachine
Copy link
Copy Markdown
Contributor

💚 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

kibanamachine added a commit that referenced this pull request Jan 28, 2025
…dashboard` (#208165) (#208604)

# Backport

This will backport the following commits from `main` to `8.x`:
- [Remove TS reference from `stack_connectors` to
`dashboard`
(#208165)](#208165)

<!--- Backport version: 9.4.3 -->

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

<!--BACKPORT [{"author":{"name":"Pierre
Gayvallet","email":"pierre.gayvallet@elastic.co"},"sourceCommit":{"committedDate":"2025-01-28T18:58:57Z","message":"Remove
TS reference from `stack_connectors` to `dashboard` (#208165)\n\n##
Summary\r\n\r\nIn #159075 was introduced a reference from the
`stack_connectors` plugin\r\nto the `dashboard` plugin. This dependency
was made so that the logic\r\ncreating the GenAI dashboard has proper
typing for the dashboard SO\r\nattributes type.\r\n\r\nHowever, that is
a bad isolation of concerns, introducing a high risk of\r\ncyclic
dependencies: the `dashboard` plugin, or any of its
dependencies,\r\ncan't depend on `stack_connectors`, or any plugin
depending on\r\nstack_connectors.\r\n\r\nThis is especially problematic
because of another bad design decision of\r\nhaving `dashboard`
depending on `observability_ai_assistant`, as it\r\nincreases the risk
of circular dependencies.\r\n\r\nThis PR addresses it by removing the
few usages we have of the dashboard\r\nplugin's type in
stack_connectors. Of course the ideal fix would be to\r\nextract the
type from the dashboard plugin instead, but those types\r\nwe're using
here are inferred from config schema concrete objects,\r\nmaking it
complicated, so I went the pragmatic road even if it means\r\nloosing a
bit of type
safety.","sha":"5b6261782e6c93ccd11c3b145cc01bf671780229","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","backport:version","v8.18.0"],"title":"Remove
TS reference from `stack_connectors` to
`dashboard`","number":208165,"url":"https://github.com/elastic/kibana/pull/208165","mergeCommit":{"message":"Remove
TS reference from `stack_connectors` to `dashboard` (#208165)\n\n##
Summary\r\n\r\nIn #159075 was introduced a reference from the
`stack_connectors` plugin\r\nto the `dashboard` plugin. This dependency
was made so that the logic\r\ncreating the GenAI dashboard has proper
typing for the dashboard SO\r\nattributes type.\r\n\r\nHowever, that is
a bad isolation of concerns, introducing a high risk of\r\ncyclic
dependencies: the `dashboard` plugin, or any of its
dependencies,\r\ncan't depend on `stack_connectors`, or any plugin
depending on\r\nstack_connectors.\r\n\r\nThis is especially problematic
because of another bad design decision of\r\nhaving `dashboard`
depending on `observability_ai_assistant`, as it\r\nincreases the risk
of circular dependencies.\r\n\r\nThis PR addresses it by removing the
few usages we have of the dashboard\r\nplugin's type in
stack_connectors. Of course the ideal fix would be to\r\nextract the
type from the dashboard plugin instead, but those types\r\nwe're using
here are inferred from config schema concrete objects,\r\nmaking it
complicated, so I went the pragmatic road even if it means\r\nloosing a
bit of type
safety.","sha":"5b6261782e6c93ccd11c3b145cc01bf671780229"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/208165","number":208165,"mergeCommit":{"message":"Remove
TS reference from `stack_connectors` to `dashboard` (#208165)\n\n##
Summary\r\n\r\nIn #159075 was introduced a reference from the
`stack_connectors` plugin\r\nto the `dashboard` plugin. This dependency
was made so that the logic\r\ncreating the GenAI dashboard has proper
typing for the dashboard SO\r\nattributes type.\r\n\r\nHowever, that is
a bad isolation of concerns, introducing a high risk of\r\ncyclic
dependencies: the `dashboard` plugin, or any of its
dependencies,\r\ncan't depend on `stack_connectors`, or any plugin
depending on\r\nstack_connectors.\r\n\r\nThis is especially problematic
because of another bad design decision of\r\nhaving `dashboard`
depending on `observability_ai_assistant`, as it\r\nincreases the risk
of circular dependencies.\r\n\r\nThis PR addresses it by removing the
few usages we have of the dashboard\r\nplugin's type in
stack_connectors. Of course the ideal fix would be to\r\nextract the
type from the dashboard plugin instead, but those types\r\nwe're using
here are inferred from config schema concrete objects,\r\nmaking it
complicated, so I went the pragmatic road even if it means\r\nloosing a
bit of type
safety.","sha":"5b6261782e6c93ccd11c3b145cc01bf671780229"}},{"branch":"8.x","label":"v8.18.0","branchLabelMappingKey":"^v8.18.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Pierre Gayvallet <pierre.gayvallet@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:version Backport to applied version labels release_note:skip Skip the PR/issue when compiling release notes v8.18.0 v9.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants