Skip to content

Revert journey ergonomics PR#143268

Closed
lizozom wants to merge 4 commits intoelastic:mainfrom
lizozom:revert-ergo
Closed

Revert journey ergonomics PR#143268
lizozom wants to merge 4 commits intoelastic:mainfrom
lizozom:revert-ergo

Conversation

@lizozom
Copy link
Copy Markdown
Contributor

@lizozom lizozom commented Oct 13, 2022

Summary

Revert #140680
Due to #141420

Checklist

Delete any items that are not applicable to this PR.

Risk Matrix

Delete this section if it is not applicable to this PR.

Before closing this PR, invite QA, stakeholders, and other developers to identify risks that should be tested prior to the change/feature release.

When forming the risk matrix, consider some of the following examples and how they may potentially impact the change:

Risk Probability Severity Mitigation/Notes
Multiple Spaces—unexpected behavior in non-default Kibana Space. Low High Integration tests will verify that all features are still supported in non-default Kibana Space and when user switches between spaces.
Multiple nodes—Elasticsearch polling might have race conditions when multiple Kibana nodes are polling for the same tasks. High Low Tasks are idempotent, so executing them multiple times will not result in logical error, but will degrade performance. To test for this case we add plenty of unit tests around this logic and document manual testing procedure.
Code should gracefully handle cases when feature X or plugin Y are disabled. Medium High Unit tests will verify that any feature flag or plugin combination still results in our service operational.
See more potential risk examples

For maintainers

@lizozom lizozom changed the title revert Revert journey ergonomics PR Oct 13, 2022
lizozom and others added 3 commits October 13, 2022 22:06
Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>
@lizozom lizozom marked this pull request as ready for review October 18, 2022 10:12
@lizozom lizozom requested review from a team as code owners October 18, 2022 10:12
@lizozom lizozom added the release_note:skip Skip the PR/issue when compiling release notes label Oct 18, 2022
@kibana-ci
Copy link
Copy Markdown

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
@kbn/dev-cli-runner 85 64 -21
@kbn/ftr-common-functional-services 28 - -28
@kbn/journeys 59 - -59
@kbn/test 217 213 -4
total -112

Any counts in public APIs

Total count of every any typed public API. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats any for more detailed information.

id before after diff
@kbn/test 4 5 +1

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id before after diff
@kbn/ftr-common-functional-services 2 - -2
@kbn/journeys 5 - -5
total -7
Unknown metric groups

API count

id before after diff
@kbn/dev-cli-runner 101 65 -36
@kbn/ftr-common-functional-services 28 - -28
@kbn/journeys 64 - -64
@kbn/test 261 254 -7
total -135

ESLint disabled in files

id before after diff
@kbn/failed-test-reporter-cli 1 - -1
@kbn/test 1 3 +2
total +1

ESLint disabled line counts

id before after diff
@kbn/journeys 1 - -1

Total ESLint disabled count

id before after diff
@kbn/failed-test-reporter-cli 1 - -1
@kbn/journeys 1 - -1
@kbn/test 8 10 +2
total -0

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@dmlemeshko
Copy link
Copy Markdown
Contributor

I spinned up a build with performance tests for this branch and it looks to be ok: tests passed, data extractor produced valid json files. I didn't check events yet.

Copy link
Copy Markdown
Contributor

@wayneseymour wayneseymour left a comment

Choose a reason for hiding this comment

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

Code only review lgtm

@lizozom
Copy link
Copy Markdown
Contributor Author

lizozom commented Oct 19, 2022

Closing for now following discussion with @spenceralger.
We'll try to resolve the integration issues with stress tests and any others that may arise.

@lizozom lizozom closed this Oct 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release_note:skip Skip the PR/issue when compiling release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants