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.

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

Use non deprecated xcontenthelper #28503

Merged
merged 3 commits into from
Feb 5, 2018

Conversation

dakrone
Copy link
Member

@dakrone dakrone commented Feb 2, 2018

This moves away from one of the now-deprecated XContentHelper.createParser
methods in favor of specifying the deprecation logger at parser creation time.

Relates to #28449

Note that this doesn't move all the createParser calls because some of them
use the already-deprecated method that doesn't specify the XContentType.

This moves away from one of the now-deprecated XContentHelper.createParser
methods in favor of specifying the deprecation logger at parser creation time.

Relates to elastic#28449

Note that this doesn't move all the `createParser` calls because some of them
use the already-deprecated method that doesn't specify the XContentType.
Copy link
Member

@nik9000 nik9000 left a comment

Choose a reason for hiding this comment

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

I left a note about indentation. Can you add four more spaces to all the try changes?

@@ -316,7 +317,8 @@ static Request bulk(BulkRequest bulkRequest) throws IOException {
BytesReference indexSource = indexRequest.source();
XContentType indexXContentType = indexRequest.getContentType();

try (XContentParser parser = XContentHelper.createParser(NamedXContentRegistry.EMPTY, indexSource, indexXContentType)) {
try (XContentParser parser = XContentHelper.createParser(NamedXContentRegistry.EMPTY,
LoggingDeprecationHandler.INSTANCE, indexSource, indexXContentType)) {
Copy link
Member

Choose a reason for hiding this comment

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

Can you indent this one more time? The way it is the method args look like they are part of the try body.

Copy link
Member Author

Choose a reason for hiding this comment

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

I checked but I thought we used what Intellij thought the indentation should be? (I even opened Intellij to make sure it agreed)

Copy link
Member

Choose a reason for hiding this comment

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

I tend to disagree with IntelliJ here. It is hard to read..... Whatever works though.

@dakrone
Copy link
Member Author

dakrone commented Feb 5, 2018

@elasticmachine test this again please

@dakrone dakrone merged commit eebff4d into elastic:master Feb 5, 2018
dakrone added a commit that referenced this pull request Feb 5, 2018
* Move to non-deprecated XContentHelper.createParser(...)

This moves away from one of the now-deprecated XContentHelper.createParser
methods in favor of specifying the deprecation logger at parser creation time.

Relates to #28449

Note that this doesn't move all the `createParser` calls because some of them
use the already-deprecated method that doesn't specify the XContentType.

* Remove the deprecated (and now non-needed) createParser method
martijnvg added a commit that referenced this pull request Feb 7, 2018
* es/master:
  Added more parameter to PersistentTaskPlugin#getPersistentTasksExecutor(...)
  [Tests] Relax assertion in SuggestStatsIT (#28544)
  Make internal Rounding fields final (#28532)
  Fix the ability to remove old plugin
  [TEST] Expand failure message for wildfly integration tests
  Add 6.2.1 version constant
  Remove feature parsing for GetIndicesAction (#28535)
  No refresh on shard activation needed (#28013)
  Improve failure message when restoring an index that already exists in the cluster (#28498)
  Use right skip versions.
  [Docs] Fix incomplete URLs (#28528)
  Use non deprecated xcontenthelper (#28503)
  Painless: Fixes a null pointer exception in certain cases of for loop usage (#28506)
martijnvg added a commit that referenced this pull request Feb 7, 2018
* es/6.x:
  Added more parameter to PersistentTaskPlugin#getPersistentTasksExecutor(...)
  [Tests] Relax assertion in SuggestStatsIT (#28544)
  Make internal Rounding fields final (#28532)
  Skip verify versions for buggy cgroup2 handling
  Fix the ability to remove old plugin
  [TEST] Expand failure message for wildfly integration tests
  Add 6.2.1 version constant
  [DOCS] Adding 6.2 RNs
  [DOCS] Added entry for 6.2.0 RNs
  Remove feature parsing for GetIndicesAction (#28535)
  No refresh on shard activation needed (#28013)
  Improve failure message when restoring an index that already exists in the cluster (#28498)
  testIndexCausesIndexCreation should not use the `_primary` preference
  Use right skip versions.
  [Docs] Fix incomplete URLs (#28528)
  Use non deprecated xcontenthelper (#28503)
  Painless: Fixes a null pointer exception in certain cases of for loop usage (#28506)
@dakrone dakrone deleted the use-non-deprecated-xcontenthelper branch April 19, 2018 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants