k$ -> observable.pipe #4

kimjoar · 2017-11-01T10:58:05Z

Hm, how bad would it be to just put a pipe method on the “native” Observable in the observable PR? We’re still basically exposing a native observable, plus it cleans up the api sooo much compared to the k$(...)(...) stuff we’re doing now.

It basically looks like “lettable operators” in RxJS, just that it’s on a “mostly” native Observable. Handling the tiny pipe diff when updating the native observable should be super-easy too.

(I added a section about "Why pipe" in the kbn-observable readme in this PR)

kimjoar · 2017-11-01T10:58:55Z

packages/kbn-internal-native-observable/index.js

+      return this;
+    }
+
+    return operations.reduce((prev, fn) => fn(prev), this);


this is the implementation of pipe ^, which is the only needed change here. Otherwise we're just adding the types to the type declarations plus switching over everything from k$

rhoboat · 2017-11-01T11:04:21Z

I really like this. I liked it in that article and wondered if we could use it. ++

epixa · 2017-11-03T16:35:49Z

This seems like a step backwards. I'm not concerned about the possibility of us having to change code in the future as the observable spec changes, but I am concerned about us adding a new feature to our observables that isn't likely to be available in native observables. Interoperability is a critical component of the new platform, but this proposal exempts observables from that goal, which is a huge deal since observables are such an important aspect of consuming plugin contracts.

We can draw on our experience with similar API additions that caused us a lot of heartache: native promises don't have a finally even though that is undoubtably a useful api that basically every userland promise implementation has. We used bluebird in place of native promises for a long time throughout the codebase, and so use of finally was systemic. At some point in the last year or so, we decided to replace bluebird promises in programmatic apis with native promises, and the effort was a painful and bug prone because we were so heavily and inconsistently relying on finally.

kimjoar · 2017-11-03T16:46:06Z

Well, it's good that Promise.finally is stage 3, then ;)

To me it comes down to a pragmatic choice. The k$()() thingy is really ugly when you're working with it, and we haven't come up with a better api. I think it's the wrong choice, but I'll close this, and rather bring it up later on if we don't like k$ when we start using it more.

* add tutorial directory and home promo section * tutorial components * use KuiCodeEditor for displaying instruction code * move spec files to server so joi can be used. Fetch via rest API * Adding more tutorials (#4) * More edits on the Apache logs tutorial * Added nginx, mysql, and sytem modules for FB * Moved apache to apacheLogs and added an apacheMetrics tutorial * Added mysqlMetrics, nginxMetrics, systemMetrics tutorials * Reduce duplication in the tutorials This uses common objects for the install and start steps for Filebeat. Same can be done for MB. * fix windows path for config file * add markdown parsing * use mustache to replace config.kibana.version with kibana version * add image preview to tutorial introduction * fix css class name * add param types constants * add docs variables * [WIP] Logstash Netflow module tutorial (#5) * First draft of Logstash Netflow module tutorial * Incorporated writing style suggestions * pass params to template replace logic * parameter inputs * use isReadOnly flag from ui-framework for KuiCodeEditor * dedemorton commits on netflow * remove trailing slash from base links * add config.docs.logstash and fix vertical spacing between Content component and commands * Use logstash docs config variable * Starting to add Deb instructions * Fix Logstash documentation link * Make commands optional * Refactor: extract common LS instructions * Edits for the existing tutorials * change schema to support three instruction types * [Netflow tutorial] Simplify OSX instructions * replace axios with fetch * pass credentials to tutorial fetch * display cloud instructions when cloud set * RadioButtonGroup component * update copy * add tutorial component jest tests * content component test * load isCloudEnabled in home_app * add functional test ensuring add data tutorials are fetch and displayed * rename card btns to 'Add data', fix type in tutorial directory tab, remove 'Set up index pattern from tutorial directory' * move parameters form to right of instruction set title * add copy snippet button, remove line numbers from command block * add breadcrumb to tutorial view * update tutorial jest snapshot * use componentDidMount and ignore async results if componenent has been unmounted * define shape of prop for instructionVariants and params. Create NumberParameter and StringParameter components * add bread crumb to add data directory page * Add cloud version of the apache_logs tutorial (elastic#16) * Add cloud version of the apache_logs tutorial * Added onprem-cloud version as well * fix styling broken by EUI rebase * add artifacts to tutorial schema * fix styling for code block * [Tutorials] Netflow: instructions for onPremCloud (elastic#18) * Extract common on-prem cloud instructions so LS and Beats can share them * Extracting common instructions; adding onPremCloud instructions * fix copy bug where copy would only contain previously selected text * make string parameter input type be text * Implementing Elastic Cloud tutorial for Netflow module (elastic#19) * More tutorial edits (elastic#20) * More tutorial edits This updates the on prem instructions with a step that installs the GeoIP and UA plugins in ES. It also makes the on-prem steps more consistent with the cloud instructions which results in less redundancy in the code. * Show config step for all variants * More DRY in the tutorial content * Updated screenshot for apache_logs * wrap markdown content in markdown-body class * use EuiFlexGroup to remove wasted space with 'copy snippet' button and instruction text * change OSX to macOS, use Computed property names for instruction_variant DISPLAY_MAP, replace /app/kibana with kbnBaseUrl, remove unneeded if check in copy_to_clippboard, put getTutorials mixin on server instead of request * capitilize 'C' in Elastic Cloud * remove try/catch from copy_to_clipboard * Removing unrelated instructions * Copy editing fixes * Multiply edits to the Beats tutorials (elastic#21) * Updated Nginx module * Updated MySQL logs module * Updated system logs module * Correct the on_prem_cloud enable steps * Updated the Nginx metrics tutorial and added screenshot * Updated the Apache metrics module + screenshot * Updated the MySQL metrics module + screenshot * Updated the system metrics module + screenshot * prevent 'Copy snippet' button text from wrapping * [Netflow tutorial] Windows instructions (elastic#22) * [Netflow tutorial] Adding onPrem instructions for Windows * Removing unrelated/superfluous instructions * Adding Windows instructions for onPremElasticCloud and elasticCloud * use EuiImage so tutorial images are clickable to view in full screen * fix jest tests * set fullScreenIconColor and alt options for EuiImage, add space between command block and instruction text

* Kibana Home page - phase two (elastic#14749) * add tutorial directory and home promo section * tutorial components * use KuiCodeEditor for displaying instruction code * move spec files to server so joi can be used. Fetch via rest API * Adding more tutorials (#4) * More edits on the Apache logs tutorial * Added nginx, mysql, and sytem modules for FB * Moved apache to apacheLogs and added an apacheMetrics tutorial * Added mysqlMetrics, nginxMetrics, systemMetrics tutorials * Reduce duplication in the tutorials This uses common objects for the install and start steps for Filebeat. Same can be done for MB. * fix windows path for config file * add markdown parsing * use mustache to replace config.kibana.version with kibana version * add image preview to tutorial introduction * fix css class name * add param types constants * add docs variables * [WIP] Logstash Netflow module tutorial (#5) * First draft of Logstash Netflow module tutorial * Incorporated writing style suggestions * pass params to template replace logic * parameter inputs * use isReadOnly flag from ui-framework for KuiCodeEditor * dedemorton commits on netflow * remove trailing slash from base links * add config.docs.logstash and fix vertical spacing between Content component and commands * Use logstash docs config variable * Starting to add Deb instructions * Fix Logstash documentation link * Make commands optional * Refactor: extract common LS instructions * Edits for the existing tutorials * change schema to support three instruction types * [Netflow tutorial] Simplify OSX instructions * replace axios with fetch * pass credentials to tutorial fetch * display cloud instructions when cloud set * RadioButtonGroup component * update copy * add tutorial component jest tests * content component test * load isCloudEnabled in home_app * add functional test ensuring add data tutorials are fetch and displayed * rename card btns to 'Add data', fix type in tutorial directory tab, remove 'Set up index pattern from tutorial directory' * move parameters form to right of instruction set title * add copy snippet button, remove line numbers from command block * add breadcrumb to tutorial view * update tutorial jest snapshot * use componentDidMount and ignore async results if componenent has been unmounted * define shape of prop for instructionVariants and params. Create NumberParameter and StringParameter components * add bread crumb to add data directory page * Add cloud version of the apache_logs tutorial (elastic#16) * Add cloud version of the apache_logs tutorial * Added onprem-cloud version as well * fix styling broken by EUI rebase * add artifacts to tutorial schema * fix styling for code block * [Tutorials] Netflow: instructions for onPremCloud (elastic#18) * Extract common on-prem cloud instructions so LS and Beats can share them * Extracting common instructions; adding onPremCloud instructions * fix copy bug where copy would only contain previously selected text * make string parameter input type be text * Implementing Elastic Cloud tutorial for Netflow module (elastic#19) * More tutorial edits (elastic#20) * More tutorial edits This updates the on prem instructions with a step that installs the GeoIP and UA plugins in ES. It also makes the on-prem steps more consistent with the cloud instructions which results in less redundancy in the code. * Show config step for all variants * More DRY in the tutorial content * Updated screenshot for apache_logs * wrap markdown content in markdown-body class * use EuiFlexGroup to remove wasted space with 'copy snippet' button and instruction text * change OSX to macOS, use Computed property names for instruction_variant DISPLAY_MAP, replace /app/kibana with kbnBaseUrl, remove unneeded if check in copy_to_clippboard, put getTutorials mixin on server instead of request * capitilize 'C' in Elastic Cloud * remove try/catch from copy_to_clipboard * Removing unrelated instructions * Copy editing fixes * Multiply edits to the Beats tutorials (elastic#21) * Updated Nginx module * Updated MySQL logs module * Updated system logs module * Correct the on_prem_cloud enable steps * Updated the Nginx metrics tutorial and added screenshot * Updated the Apache metrics module + screenshot * Updated the MySQL metrics module + screenshot * Updated the system metrics module + screenshot * prevent 'Copy snippet' button text from wrapping * [Netflow tutorial] Windows instructions (elastic#22) * [Netflow tutorial] Adding onPrem instructions for Windows * Removing unrelated/superfluous instructions * Adding Windows instructions for onPremElasticCloud and elasticCloud * use EuiImage so tutorial images are clickable to view in full screen * fix jest tests * set fullScreenIconColor and alt options for EuiImage, add space between command block and instruction text * fix broken depenencies (elastic#15960) * update yarn.lock

kimjoar commented Nov 1, 2017

View reviewed changes

kimjoar force-pushed the explore/kbn-observable-pipe branch from 3d7ae00 to 4a36da3 Compare November 1, 2017 11:02

kimjoar force-pushed the explore/kbn-observable-pipe branch from de6f4db to e4607c2 Compare November 2, 2017 18:59

kimjoar force-pushed the platform/kbn-observable-lib branch 2 times, most recently from a0ef568 to 2588102 Compare November 2, 2017 19:02

k$ -> observable.pipe

5445057

kimjoar force-pushed the explore/kbn-observable-pipe branch 2 times, most recently from f89e942 to fa64d2e Compare November 3, 2017 13:39

Update readme about k$ and pipe

e038e0b

kimjoar force-pushed the explore/kbn-observable-pipe branch from fa64d2e to e038e0b Compare November 3, 2017 13:40

kimjoar closed this Nov 3, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

k$ -> observable.pipe #4

k$ -> observable.pipe #4

kimjoar commented Nov 1, 2017 •

edited

Loading

kimjoar Nov 1, 2017 •

edited

Loading

rhoboat commented Nov 1, 2017

epixa commented Nov 3, 2017

kimjoar commented Nov 3, 2017

k$ -> observable.pipe #4

k$ -> observable.pipe #4

Conversation

kimjoar commented Nov 1, 2017 • edited Loading

kimjoar Nov 1, 2017 • edited Loading

Choose a reason for hiding this comment

rhoboat commented Nov 1, 2017

epixa commented Nov 3, 2017

kimjoar commented Nov 3, 2017

kimjoar commented Nov 1, 2017 •

edited

Loading

kimjoar Nov 1, 2017 •

edited

Loading