Skip to content

Conversation

@dnhatn
Copy link
Member

@dnhatn dnhatn commented May 14, 2018

This commit addresses some comments given after the original PR was in.

Follow-up #30400 (comment)

This commit addresses some comments given after the original PR was in.

Follow-up elastic#30400
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

@dnhatn dnhatn requested a review from javanna May 14, 2018 19:53
@dnhatn
Copy link
Member Author

dnhatn commented May 14, 2018

Hey @javanna, this addresses your comments in #30400 (comment) . Can you please have a look? Thank you!

Copy link
Member

@javanna javanna left a comment

Choose a reason for hiding this comment

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

left a tiny comment, LGTM otherwise

* random instance each time it is called.
*/
protected abstract T createTestInstance();
protected abstract T createTestInstance() throws IOException;
Copy link
Member

Choose a reason for hiding this comment

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

I think I would prefer to catch the exception in createTestInstance instead. We should not need this in most of the cases, and it would be nice to keep this aligned with AbstractWireTestCase#createTestInstance

@dnhatn
Copy link
Member Author

dnhatn commented May 15, 2018

@elasticmachine test this please

@dnhatn
Copy link
Member Author

dnhatn commented May 15, 2018

Thanks @javanna

@dnhatn dnhatn merged commit d1c28c6 into elastic:master May 15, 2018
@dnhatn dnhatn deleted the put-template-followup branch May 15, 2018 12:07
dnhatn added a commit that referenced this pull request May 15, 2018
This commit addresses some comments given after the original PR was in.

Follow-up #30400
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request May 15, 2018
…ngs-to-true

* elastic/master:
  [DOCS] Restores 7.0.0 release notes and highlights
  Remove assert statements from field caps documentation. (elastic#30601)
  Repository GCS plugin new client library (elastic#30168)
  HLRestClient: Follow-up for put index template api (elastic#30592)
  Unmute IndexUpgradeIT tests
  [DOCS] Remove references to changelog and to highlights
  Side-step pending deletes check (elastic#30571)
  [DOCS] Remove references to removed changelog
  Revert "Mute ML upgrade test (elastic#30458)"
  [ML] Adjust BWC version following backport of elastic#30125
  [Docs] Improve section detailing translog usage (elastic#30573)
  [Tests] Relax allowed delta in extended_stats aggregation (elastic#30569)
  [ML] Reverse engineer Grok patterns from categorization results (elastic#30125)
  Update build file due to doc file rename
  Remove the changelog (elastic#30593)
  Fix issue with finishing handshake in ssl driver (elastic#30580)
  Fail if reading from closed KeyStoreWrapper (elastic#30394)
  Silence IndexUpgradeIT test failures. (elastic#30430)
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.

4 participants