Add RFC for Rest API as commands. #42

VijayanB · 2021-02-10T03:15:57Z

Added rfc for rest api as commands

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

codecov · 2021-02-10T03:19:04Z

Codecov Report

Merging #42 (14bc15a) into main (b1c86fe) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main      #42   +/-   ##
=======================================
  Coverage   65.33%   65.33%           
=======================================
  Files          20       20           
  Lines        1255     1255           
=======================================
  Hits          820      820           
  Misses        341      341           
  Partials       94       94

Flag	Coverage Δ
odfe-cli	`65.33% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

jmazanec15 · 2021-02-10T18:32:48Z

docs/dev/rfc/odfe-cli-rest-api-as-commands.md

+## Objective
+
+This document describes how to support [rest api](https://www.elastic.co/guide/en/elasticsearch/reference/current/rest-apis.html) in a generic way.
+As of now, elasticsearch support api in different categories like index api, cat api, cluster api, document api, etc to


Capitalize "e" in elasticsearch.

jmazanec15 · 2021-02-10T18:33:23Z

docs/dev/rfc/odfe-cli-rest-api-as-commands.md

+This document describes how to support [rest api](https://www.elastic.co/guide/en/elasticsearch/reference/current/rest-apis.html) in a generic way.
+As of now, elasticsearch support api in different categories like index api, cat api, cluster api, document api, etc to
+configure and access their features. It will take tremendous amount of time and effort to provide every api as native commands.
+Hence, we will be categorizing those REST API based on GET/PUT/POST/DELETE  and allow users to perform their request 


We should be consistent in capitalization of "rest" and "api"

I still see some incosistencies where there is "api" and "API"

jmazanec15 · 2021-02-10T18:34:01Z

docs/dev/rfc/odfe-cli-rest-api-as-commands.md

+As of now, elasticsearch support api in different categories like index api, cat api, cluster api, document api, etc to
+configure and access their features. It will take tremendous amount of time and effort to provide every api as native commands.
+Hence, we will be categorizing those REST API based on GET/PUT/POST/DELETE  and allow users to perform their request 
+in a generic way. This will support any future API without any additional support. It also benefits plugin owners while on-boarding their plugin. 


Benefitting plugin owners isn't exactly clear to me. How will this change anything?

So right now, plug in owners has to build the gateway layer by using framework methods like build request/ call request. Now, they can call ES's controller methods(get/put/post) directly .

Can you add that explanation here?

on second thought, removed this point. I will update our developer guideline to include this benefit once i merge this feature, since we need to provide a new direction on how to use this in framework. I am planning to keep those discussion away from this feature at this point.

jmazanec15 · 2021-02-10T18:34:34Z

docs/dev/rfc/odfe-cli-rest-api-as-commands.md

+
+## Common parameters
+
+The following options can be applied to all of the odfe-cli commands. These parameters are supported by server as mentioned [here](https://www.elastic.co/guide/en/elasticsearch/reference/current/common-options.html).


What do you mean "supported by server"?

i was trying to distant our cli with elasticsearch as much as possible. May be it is confusing. Let me revert back to call elasticsearch to avoid this confusion.

jmazanec15 · 2021-02-10T18:35:12Z

docs/dev/rfc/odfe-cli-rest-api-as-commands.md

+## Common parameters
+
+The following options can be applied to all of the odfe-cli commands. These parameters are supported by server as mentioned [here](https://www.elastic.co/guide/en/elasticsearch/reference/current/common-options.html).
+In future  more parameters or include multiple values will be supported that can be added as flag to any commands.


"In the future, we will support ..."

docs/dev/rfc/odfe-cli-rest-api-as-commands.md

jmazanec15 · 2021-02-10T18:36:00Z

docs/dev/rfc/odfe-cli-rest-api-as-commands.md

+
+### Description
+
+Use GET API to execute requests against elasticsearch cluster. This command enables you to run any GET based REST API commands across all 


Capitalize elasticsearch

jmazanec15 · 2021-02-10T18:37:44Z

docs/dev/rfc/odfe-cli-rest-api-as-commands.md

+2.  To  returns the health status of a cluster.
+
+```
+odfe-cli cmd get --path "_cluster/health" --pretty


I like curl instead of cmd

sure. That is one of the question in RFC. If we don't get any response, i was thinking of posting on es-learning group to get vote!

docs/dev/rfc/odfe-cli-rest-api-as-commands.md

jmazanec15 · 2021-02-12T18:26:34Z

docs/dev/rfc/odfe-cli-rest-api-as-commands.md

+This document describes how to support [rest api](https://www.elastic.co/guide/en/elasticsearch/reference/current/rest-apis.html) in a generic way.
+As of now, elasticsearch support api in different categories like index api, cat api, cluster api, document api, etc to
+configure and access their features. It will take tremendous amount of time and effort to provide every api as native commands.
+Hence, we will be categorizing those REST API based on GET/PUT/POST/DELETE  and allow users to perform their request 


I still see some incosistencies where there is "api" and "API"

jmazanec15 · 2021-02-12T18:27:19Z

docs/dev/rfc/odfe-cli-rest-api-as-commands.md

+As of now, elasticsearch support api in different categories like index api, cat api, cluster api, document api, etc to
+configure and access their features. It will take tremendous amount of time and effort to provide every api as native commands.
+Hence, we will be categorizing those REST API based on GET/PUT/POST/DELETE  and allow users to perform their request 
+in a generic way. This will support any future API without any additional support. It also benefits plugin owners while on-boarding their plugin. 


Can you add that explanation here?

docs/dev/rfc/odfe-cli-rest-api-as-commands.md

jmazanec15 · 2021-02-12T18:28:14Z

docs/dev/rfc/odfe-cli-rest-api-as-commands.md

+epoch         timestamp        count
+1612311592    12:24:24          100
+```
+2.  To  returns the health status of a cluster.


return not returns. Remove period at end of line.

docs/dev/rfc/odfe-cli-rest-api-as-commands.md

jmazanec15 · 2021-02-12T18:29:28Z

docs/dev/rfc/odfe-cli-rest-api-as-commands.md

+ ```
+ $ odfe-cli curl post --path        "_search" \
+                     --headers     "Content-Encoding : gzip;Accept-Encoding: gzip, deflate" \
+                     --data-bianry  @/tmp/req.txt.gz`


binary misspelled

vamshin

LGTM! Looking forward to this feature :)

vamshin · 2021-02-10T03:22:08Z

docs/dev/rfc/odfe-cli-rest-api-as-commands.md

+## Objective
+
+This document describes how to support [rest api](https://www.elastic.co/guide/en/elasticsearch/reference/current/rest-apis.html) in a generic way.
+As of now, elasticsearch support api in different categories like index api, cat api, cluster api, document api, etc to


Minor elasticsearch -> Elasticsearch? In other places as well

VijayanB added the Documentation Improvements or additions to documentation label Feb 10, 2021

VijayanB requested review from jmazanec15 and vamshin February 10, 2021 03:15

VijayanB self-assigned this Feb 10, 2021

jmazanec15 reviewed Feb 10, 2021

View reviewed changes

VijayanB force-pushed the rfc branch 2 times, most recently from 8f1f20a to 7b03c5c Compare February 10, 2021 20:51

VijayanB requested a review from jmazanec15 February 10, 2021 20:51

VijayanB force-pushed the rfc branch from 0d69207 to 7127bcf Compare February 11, 2021 00:30

Add RFC for Rest API as commands.

72ae959

VijayanB force-pushed the rfc branch from 7127bcf to 72ae959 Compare February 11, 2021 00:48

jmazanec15 reviewed Feb 12, 2021

View reviewed changes

vamshin approved these changes Feb 12, 2021

View reviewed changes

fixed review

1fea4b1

VijayanB force-pushed the rfc branch from cdee1ca to 1fea4b1 Compare February 12, 2021 19:16

VijayanB requested a review from jmazanec15 February 12, 2021 19:20

jmazanec15 approved these changes Feb 12, 2021

View reviewed changes

VijayanB merged commit 0f2e9ca into opendistro-for-elasticsearch:main Feb 12, 2021

VijayanB deleted the rfc branch February 12, 2021 19:48

peterzhuamazon added this to In progress in ODFE-Roadmap | https://discuss.opendistrocommunity.dev | https://opendistro.github.io Mar 16, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add RFC for Rest API as commands. #42

Add RFC for Rest API as commands. #42

VijayanB commented Feb 10, 2021

codecov bot commented Feb 10, 2021 •

edited

Loading

jmazanec15 Feb 10, 2021

VijayanB Feb 10, 2021

jmazanec15 Feb 10, 2021

VijayanB Feb 10, 2021

jmazanec15 Feb 12, 2021

jmazanec15 Feb 10, 2021

VijayanB Feb 10, 2021 •

edited

Loading

jmazanec15 Feb 12, 2021

VijayanB Feb 12, 2021

jmazanec15 Feb 10, 2021

VijayanB Feb 10, 2021

jmazanec15 Feb 10, 2021

VijayanB Feb 10, 2021

jmazanec15 Feb 10, 2021

VijayanB Feb 10, 2021

jmazanec15 Feb 10, 2021

VijayanB Feb 10, 2021

jmazanec15 Feb 12, 2021

jmazanec15 Feb 12, 2021

jmazanec15 Feb 12, 2021

VijayanB Feb 12, 2021

VijayanB Feb 12, 2021

jmazanec15 Feb 12, 2021

VijayanB Feb 12, 2021

VijayanB Feb 12, 2021

vamshin left a comment

vamshin Feb 10, 2021


		## Common parameters

		The following options can be applied to all of the odfe-cli commands. These parameters are supported by server as mentioned [here](https://www.elastic.co/guide/en/elasticsearch/reference/current/common-options.html).


		### Description

		Use GET API to execute requests against elasticsearch cluster. This command enables you to run any GET based REST API commands across all

Add RFC for Rest API as commands. #42

Add RFC for Rest API as commands. #42

Conversation

VijayanB commented Feb 10, 2021

codecov bot commented Feb 10, 2021 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

VijayanB Feb 10, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vamshin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Feb 10, 2021 •

edited

Loading

VijayanB Feb 10, 2021 •

edited

Loading