Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

Add support for ES REST API as generic commands #43

Merged
merged 7 commits into from
Feb 23, 2021

Conversation

VijayanB
Copy link
Member

Added gateway, controller to represent any REST API and execute.

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

@VijayanB VijayanB added odfe-cli tool itself Features change that introduces a new unit of functionality that satisfies a requirement labels Feb 13, 2021
@VijayanB VijayanB self-assigned this Feb 13, 2021
@VijayanB VijayanB changed the title Add support to ES REST API as generic commands Add support for ES REST API as generic commands Feb 13, 2021
@codecov
Copy link

codecov bot commented Feb 13, 2021

Codecov Report

Merging #43 (a16b017) into main (3419c3c) will increase coverage by 0.79%.
The diff coverage is 72.36%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #43      +/-   ##
==========================================
+ Coverage   64.74%   65.54%   +0.79%     
==========================================
  Files          21       28       +7     
  Lines        1268     1457     +189     
==========================================
+ Hits          821      955     +134     
- Misses        353      404      +51     
- Partials       94       98       +4     
Flag Coverage Δ
odfe-cli 65.54% <72.36%> (+0.79%) ⬆️

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

Impacted Files Coverage Δ
commands/curl.go 18.60% <18.60%> (ø)
gateway/es/es.go 72.72% <68.42%> (-4.20%) ⬇️
commands/curl_delete.go 75.00% <75.00%> (ø)
commands/curl_get.go 80.00% <80.00%> (ø)
commands/curl_post.go 80.00% <80.00%> (ø)
commands/curl_put.go 80.00% <80.00%> (ø)
mapper/es/es.go 96.77% <96.77%> (ø)
controller/es/es.go 100.00% <100.00%> (ø)
gateway/ad/ad.go 60.37% <100.00%> (ø)
gateway/knn/knn.go 69.04% <100.00%> (ø)
... and 8 more

Added curl, get, put, post, delete commands
to execute appropriate REST Action.
Added integration tests.
Since  elasticsearch can support output format as json/yaml,
allow user to add it as argument. if no value is passed, default by elasticsearch
will be returned.
curl command  accept a filter_path parameter that can be used to filter the response returned by Elasticsearch.
This parameter takes a comma separated list of filters.
commands/curl.go Outdated
//curlCommand is base command for Elasticsearch REST APIs.
var curlCommand = &cobra.Command{
Use: curlCommandName,
Short: "Manage elasticsearch core features",
Copy link
Member

Choose a reason for hiding this comment

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

nit: Capitalize elasticsearch

Copy link
Member Author

Choose a reason for hiding this comment

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

Ack

commands/curl.go Outdated
var curlCommand = &cobra.Command{
Use: curlCommandName,
Short: "Manage elasticsearch core features",
Long: "Use curl command to configure and access features directly",
Copy link
Member

Choose a reason for hiding this comment

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

rephrase: "Use the curl command to execute any REST API calls against the cluster."

Copy link
Member Author

Choose a reason for hiding this comment

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

Ack

commands/curl.go Show resolved Hide resolved
commands/curl.go Show resolved Hide resolved
commands/curl_delete.go Show resolved Hide resolved
var curlPutExample = `
# Create a knn index from mapping setting saved in file "knn-mapping.json"
odfe-cli curl put --path "my-knn-index" \
--data "@some-location/knn-mapping.json" \
Copy link
Member

Choose a reason for hiding this comment

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

Out of curiousity: is prefixing file input with @ a convention in another cli?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not really, usually there will be two different flags. This i borrowed from curl. Do you prefer as another flag "--data-file-path" something like that?

Copy link
Member

Choose a reason for hiding this comment

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

If this is how curl does it, thats fine

gateway/gateway.go Show resolved Hide resolved
Copy link
Member

@vamshin vamshin left a comment

Choose a reason for hiding this comment

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

LGTM! This feature opens up many use cases for community to use CLI. Thanks for working on this.

@VijayanB VijayanB merged commit 941c719 into opendistro-for-elasticsearch:main Feb 23, 2021
@VijayanB VijayanB deleted the curl branch February 23, 2021 18:53
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Features change that introduces a new unit of functionality that satisfies a requirement odfe-cli tool itself
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants