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.

Sign up for GitHub

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

Jump to bottom

[Expressions] Finish connecting debug mode on expressions #78874

Merged
merged 4 commits into from
Oct 16, 2020
Merged

[Expressions] Finish connecting debug mode on expressions #78874

merged 4 commits into from
Oct 16, 2020

Conversation

wylieconlon
Copy link
Contributor

The initial expressions debug mode PR kept the debug mode entirely internal to the expression service, making it impossible to consume debug information from Lens. By adding more connection points, we can initialize a "debuggable" request in more places, such as if we choose to add this to the Lens inspector in the future.

Summarize your PR. If it involves visual changes include a screenshot or gif.

Checklist

@wylieconlon wylieconlon added Feature:ExpressionLanguage Interpreter expression language (aka canvas pipeline) v8.0.0 Team:AppArch release_note:skip Skip the PR/issue when compiling release notes v7.11.0 labels Sep 29, 2020
@wylieconlon wylieconlon requested a review from a team as a code owner September 29, 2020 21:23
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app-arch (Team:AppArch)

ppisljar
ppisljar approved these changes Oct 5, 2020
View reviewed changes
Copy link
Member

@ppisljar ppisljar left a comment

Choose a reason for hiding this comment

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

code LGTM

@wylieconlon
Copy link
Contributor Author

@ppisljar @streamich Do you want to review again? I fixed some merge conflicts, but I'm not sure if I should be writing tests for the new arguments.

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

page load bundle size

id before after diff
expressions 198.5KB 198.6KB +113.0B

History

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

@wylieconlon
Copy link
Contributor Author

@ppisljar @streamich Since you're already approved and didn't ask for anything specific, I will merge soon.

@wylieconlon wylieconlon merged commit a5a42fd into elastic:master Oct 16, 2020
@wylieconlon wylieconlon deleted the expressions-debug-connected branch October 16, 2020 16:06
@wylieconlon wylieconlon mentioned this pull request Oct 16, 2020
Merged
wylieconlon pushed a commit to wylieconlon/kibana that referenced this pull request Oct 16, 2020
@wylieconlon
[Expressions] Finish connecting debug mode on expressions (elastic#78874
141b99b 
)

* [Expressions] Finish connecting debug mode on expressions

* Fix bad merge

* Commit API docs
wylieconlon pushed a commit that referenced this pull request Oct 16, 2020
[Expressions] Finish connecting debug mode on expressions (#78874) (#…
549ab5a 
…80862)

* [Expressions] Finish connecting debug mode on expressions

* Fix bad merge

* Commit API docs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@streamich streamich streamich approved these changes

@ppisljar ppisljar ppisljar approved these changes

@lukeelmers lukeelmers Awaiting requested review from lukeelmers

Assignees
No one assigned
Labels
Feature:ExpressionLanguage Interpreter expression language (aka canvas pipeline) release_note:skip Skip the PR/issue when compiling release notes v7.11.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@wylieconlon @elasticmachine @kibanamachine @streamich @ppisljar