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

HLRC: add client side RefreshPolicy #33209

Merged
merged 4 commits into from
Aug 30, 2018
Merged

HLRC: add client side RefreshPolicy #33209

merged 4 commits into from
Aug 30, 2018

Conversation

jaymode
Copy link
Member

@jaymode jaymode commented Aug 28, 2018

With the switch to client side request and response objects, we need a
client side version of RefreshPolicy. This change adds a client side
version of RefreshPolicy along with a method to add it to the
parameters of a request. The existing method to add
WriteRequest.RefreshPolicy to the parameters of a request is now
deprecated.

@jaymode
HLRC: add client side RefreshPolicy
1fa6ca9 
With the switch to client side request and response objects, we need a
client side version of RefreshPolicy. This change adds a client side
version of RefreshPolicy along with a method to add it to the
parameters of a request. The existing method to add
WriteRequest.RefreshPolicy to the parameters of a request is now
deprecated.
@jaymode jaymode requested a review from hub-cap August 28, 2018 16:20
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

@jaymode jaymode mentioned this pull request Aug 28, 2018
Merged
javanna
javanna reviewed Aug 28, 2018
View reviewed changes
client/rest-high-level/src/main/java/org/elasticsearch/client/RequestConverters.java
* @deprecated If creating a new HLRC ReST API call, use {@link RefreshPolicy}
* instead of {@link WriteRequest.RefreshPolicy} from the server project
*/
@Deprecated
Copy link
Member

Choose a reason for hiding this comment

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

isn't it odd that the existing oss methods use and still need the deprecated variant, and there is no alternative to it?

Copy link
Member Author

Choose a reason for hiding this comment

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

Isn't that the standard we've adopted by making the HLRC performRequest methods deprecated that take the existing request classes in #32912? The alternative is available by swapping to the POJO request/response classes that we want the HLRC to move towards?

Copy link
Member

Choose a reason for hiding this comment

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

Yea but I think there's a slight difference, but I may be missing something. The deprecations we added until now are not directly visible to users while this one is, because users are specifying the policy and they at the moment can only specify the deprecated one.

Copy link
Member Author

Choose a reason for hiding this comment

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

This isn't user facing code. It is only used in the RequestConverters class to convert a request into XContent with parameters. Users should really only be interacting with the client, request, and response objects. I could also be missing something

Copy link
Member

Choose a reason for hiding this comment

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

you are right :) that is the bit that I was missing, thanks for clarifying. all good ;)

hub-cap
hub-cap approved these changes Aug 29, 2018
View reviewed changes
Copy link
Contributor

@hub-cap hub-cap left a comment

Choose a reason for hiding this comment

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

I like what u did here, especially w the commit msgs.

@jaymode jaymode merged commit f063587 into elastic:master Aug 30, 2018
@jaymode jaymode deleted the hlrc_refresh_policy branch August 30, 2018 03:44
jaymode added a commit that referenced this pull request Aug 30, 2018
@jaymode
HLRC: add client side RefreshPolicy (#33209)
39244bb 
With the switch to client side request and response objects, we need a
client side version of RefreshPolicy. This change adds a client side
version of RefreshPolicy along with a method to add it to the
parameters of a request. The existing method to add
WriteRequest.RefreshPolicy to the parameters of a request is now
deprecated.
dnhatn added a commit that referenced this pull request Sep 1, 2018
@dnhatn
Merge branch '6.x' into ccr-6.x
a6312a5 
* 6.x:
  Mute test watcher usage stats output
  [Rollup] Fix FullClusterRestart test
  TEST: Disable soft-deletes in ParentChildTestCase
  TEST: Disable randomized soft-deletes settings
  Integrates soft-deletes into Elasticsearch (#33222)
  drop `index.shard.check_on_startup: fix` (#32279)
  Fix AwaitsFix issue number
  Mute SmokeTestWatcherWithSecurityIT testsi
  [DOCS] Moves ml folder from x-pack/docs to docs (#33248)
  TEST: mute more SmokeTestWatcherWithSecurityIT tests
  [DOCS] Move rollup APIs to docs (#31450)
  [DOCS] Rename X-Pack Commands section (#33005)
  Fixes SecurityIntegTestCase so it always adds at least one alias (#33296)
  TESTS: Fix Random Fail in MockTcpTransportTests (#33061) (#33307)
  MINOR: Remove Dead Code from PathTrie (#33280) (#33306)
  Fix pom for build-tools (#33300)
  Lazy evaluate java9home (#33301)
  SQL: test coverage for JdbcResultSet (#32813)
  Work around to be able to generate eclipse projects (#33295)
  Different handling for security specific errors in the CLI. Fix for #33230 (#33255)
  [ML] Refactor delimited file structure detection (#33233)
  SQL: Support multi-index format as table identifier (#33278)
  Enable forbiddenapis server java9 (#33245)
  [MUTE] SmokeTestWatcherWithSecurityIT flaky tests
  Add region ISO code to GeoIP Ingest plugin (#31669) (#33276)
  Don't be strict for 6.x
  Update serialization versions for custom IndexMetaData backport
  Replace IndexMetaData.Custom with Map-based custom metadata (#32749)
  Painless: Fix Bindings Bug (#33274)
  SQL: prevent duplicate generation for repeated aggs (#33252)
  TEST: Mute testMonitorClusterHealth
  Fix serialization of empty field capabilities response (#33263)
  Fix nested _source retrieval with includes/excludes (#33180)
  [DOCS] TLS file resources are reloadable (#33258)
  Watcher: Ensure TriggerEngine start replaces existing watches (#33157)
  Ignore module-info in jar hell checks (#33011)
  Fix docs build after #33241
  [DOC] Repository GCS ADC not supported (#33238)
  Upgrade to latest Gradle 4.10  (#32801)
  Fix/30904 cluster formation part2 (#32877)
  Move file-based discovery to core (#33241)
  HLRC: add client side RefreshPolicy (#33209)
  [Kerberos] Add unsupported languages for tests (#33253)
  Watcher: Reload properly on remote shard change (#33167)
  Fix classpath security checks for external tests. (#33066)
  [Rollup] Only allow aggregating on multiples of configured interval (#32052)
  Added deprecation warning for rescore in scroll queries (#33070)
  Apply settings filter to get cluster settings API (#33247)
  [Rollup] Re-factor Rollup Indexer into a generic indexer for re-usability   (#32743)
  HLRC: create base timed request class (#33216)
  HLRC: Use Optional in validation logic (#33104)
  Painless: Add Bindings (#33042)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@javanna javanna javanna left review comments

@hub-cap hub-cap hub-cap approved these changes

Assignees
No one assigned
Labels
>enhancement v6.5.0 v7.0.0-beta1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@jaymode @elasticmachine @hub-cap @javanna @colings86