Skip to content

Adds ILMClient for use with transport client #33357

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
Merged
merged 3 commits into from
Sep 5, 2018
Merged

Adds ILMClient for use with transport client #33357

merged 3 commits into from
Sep 5, 2018

Conversation

@colings86
Copy link
Contributor

@colings86 colings86 commented Sep 3, 2018

No description provided.

@colings86 colings86 added review :Data Management/ILM+SLM Index and Snapshot lifecycle management labels Sep 3, 2018
@colings86 colings86 self-assigned this Sep 3, 2018
@elasticmachine
Copy link
Collaborator

elasticmachine commented Sep 3, 2018

Pinging @elastic/es-core-infra

@elasticmachine elasticmachine mentioned this pull request Sep 3, 2018
Closed
gwbrown
gwbrown approved these changes Sep 4, 2018
View reviewed changes
Copy link
Contributor

@gwbrown gwbrown left a comment

Choose a reason for hiding this comment

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

Does this need tests? I think I recall a conversation where it was noted that transport client stuff isn't really tested, but just asking in case.

Otherwise, LGTM.

@dakrone
Copy link
Member

dakrone commented Sep 4, 2018

Are we going all async listener for this? In our other clients (like IndicesAdminClient for instance) we have versions that return ActionFuture so someone can do ExplainLifecycleResponse resp = client.explainLifecycle(req).get();

@colings86
Copy link
Contributor Author

colings86 commented Sep 4, 2018

Good point @dakrone I'll add the sync methods

@colings86
Copy link
Contributor Author

colings86 commented Sep 4, 2018

@dakrone I pushed a commit with the sync methods

dakrone
dakrone requested changes Sep 4, 2018
View reviewed changes
Copy link
Member

@dakrone dakrone left a comment

Choose a reason for hiding this comment

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

I believe this needs to be added to XPackClient similar to how we have the other MonitoringClient, WatcherClient, etc, that way it is accessible rather than instantiating it ourselves

dakrone
dakrone approved these changes Sep 4, 2018
View reviewed changes
Copy link
Member

@dakrone dakrone left a comment

Choose a reason for hiding this comment

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

LGTM, thanks Colin

@colings86 colings86 merged commit a7b5f2c into elastic:index-lifecycle Sep 5, 2018
@colings86 colings86 deleted the ilm/transport-client branch September 5, 2018 11:50
colings86 added a commit that referenced this pull request Sep 5, 2018
@colings86
Adds ILMClient for use with transport client (#33357)
6fe05bf 
* Adds ILMClient for use with transport client

* add sync methods

* adds ILMClient to XPackClient
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dakrone dakrone dakrone approved these changes

@talevy talevy Awaiting requested review from talevy

@jasontedor jasontedor Awaiting requested review from jasontedor

+1 more reviewer

@gwbrown gwbrown gwbrown approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@colings86 colings86

Labels

:Data Management/ILM+SLM Index and Snapshot lifecycle management

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@colings86 @elasticmachine @dakrone @gwbrown