Skip to content

Conversation

@javanna
Copy link
Member

@javanna javanna commented Nov 21, 2016

This is a followup to #21689 where we removed a misplaced try catch for IndexMissingException and IndexClosedException which was related to #9047 (at least for the index closed case). The code block within the change was moved as part of #20890, which made the catch redundant. It was somehow used before (e.g. in 5.0) but it doesn't seem that this catch had any effect. Added tests to verify that. In fact a specific catch added to the search api only would defeat the purpose of having common indices options that work throughout all our APIs.

Relates to #21689

@javanna javanna added :Search/Search Search-related issues that do not fall into other categories review >test Issues or PRs that are addressing/adding tests v5.1.1 v6.0.0-alpha1 labels Nov 21, 2016
@javanna
Copy link
Member Author

javanna commented Nov 21, 2016

@clintongormley could you have a quick look please?

Copy link
Contributor

@clintongormley clintongormley left a comment

Choose a reason for hiding this comment

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

Just one copy paste bug, otherwise LGTM

Copy link
Contributor

Choose a reason for hiding this comment

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

missing_index -> index_closed

Copy link
Member Author

@javanna javanna Nov 21, 2016

Choose a reason for hiding this comment

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

argh, right I will fix thanks for catching this

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed ;)

This is a followup to elastic#21689 where we removed a misplaced try catch for IndexMissingException and IndexClosedException which was related to elastic#9047 (at least for the index closed case). The code block within the change was moved as part of elastic#20890, which made the catch redundant. It was somehow used before (e.g. in 5.0) but it doesn't seem that this catch had any effect. Added tests to verify that. In fact a specific catch added to the search api only would defeat the purpose of having common indices options that work throughout all our APIs.

Relates to elastic#21689
@javanna javanna force-pushed the tests/search_indices_options branch from ea3c8cd to 3d2c350 Compare November 21, 2016 13:46
@javanna javanna merged commit c174e36 into elastic:master Nov 21, 2016
javanna added a commit that referenced this pull request Nov 21, 2016
This is a followup to #21689 where we removed a misplaced try catch for IndexMissingException and IndexClosedException which was related to #9047 (at least for the index closed case). The code block within the change was moved as part of #20890, which made the catch redundant. It was somehow used before (e.g. in 5.0) but it doesn't seem that this catch had any effect. Added tests to verify that. In fact a specific catch added to the search api only would defeat the purpose of having common indices options that work throughout all our APIs.

Relates to #21689
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Nov 22, 2016
* master: (42 commits)
  Add support for merging custom meta data in tribe node (elastic#21552)
  [DOCS] Show EC2's auto attribute (elastic#21474)
  Add information about the removal of store throttling to the migration guide.
  Add a recommendation against large documents to the docs. (elastic#21652)
  Add indices options tests to search api REST tests (elastic#21701)
  Fixing indentation in geospatial querying example. (elastic#21682)
  Fix typo in filters aggregation docs (elastic#21690)
  Add BWC layer for Exceptions (elastic#21694)
  Add checkstyle rule to forbid empty javadoc comments (elastic#20881)
  Docs: Added offline install link for discovery-file plugin
  remove pointless catch exception in TransportSearchAction (elastic#21689)
  Rename ClusterState#lookupPrototypeSafe to `lookupPrototype` and remove previous "unsafe" unused variant (elastic#21686)
  Use a buffer to do character to byte conversion in StreamOutput#writeString (elastic#21680)
  Fix integer overflows when dealing with templates. (elastic#21628)
  Fix highlighting on a stored keyword field (elastic#21645)
  Set execute permissions for native plugin programs (elastic#21657)
  adjust visibility of DiscoveryNodes.Delta constructor
  Remove unused DiscoveryNodes.Delta constructor
  Remove unused DiscoveryNode#removeDeadMembers public method
  Remove minNodeVersion and corresponding public `getSmallestVersion` getter method from DiscoveryNodes
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Search/Search Search-related issues that do not fall into other categories >test Issues or PRs that are addressing/adding tests v5.1.1 v6.0.0-alpha1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants