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

Reindex: Fixed typo in assertion failure message #30619

Merged
merged 3 commits into from
May 16, 2018
Merged

Reindex: Fixed typo in assertion failure message #30619

merged 3 commits into from
May 16, 2018
+2 −2

Conversation

ashashwat
Copy link
Contributor

@ashashwat ashashwat commented May 15, 2018 •
edited by nik9000
Loading

Fix a typo in an assertion failure message.

@nik9000 nik9000 added >non-issue :Distributed Indexing/CRUD A catch all label for issues around indexing, updating and getting a doc by id. Not search. v7.0.0 v6.4.0 labels May 15, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed

@nik9000
Copy link
Member

nik9000 commented May 15, 2018

Thanks for fixing it @ashashwat!

@elasticmachine, test this please

@nik9000 nik9000 changed the title [DOC] Fixed typo in assert comment. Reindex: Fixed typo in assertion failure message May 15, 2018
@nik9000
Copy link
Member

nik9000 commented May 15, 2018

@ashashwat I believe there is actually a type in the production code too. The tests now fail with:

org.junit.ComparisonFailure: expected:<...line] as a query par[am]eter. Specify it in ...> but was:<...line] as a query par[ma]eter. Specify it in ...>

I believe you can reproduce it with:

./gradlew :modules:reindex:test -Dtests.seed=8A50475951076F01 -Dtests.class=org.elasticsearch.index.reindex.RestReindexActionTests -Dtests.method="testPipelineQueryParameterIsError" -Dtests.security.manager=true -Dtests.locale=fr-CA -Dtests.timezone=America/Argentina/Buenos_Aires

Would you like to fix the production code as well?

@ashashwat
Copy link
Contributor Author

@nik9000 Let me go ahead and do that.

@ashashwat
Copy link
Contributor Author

@nik9000 Is it ok if I squash these commits in my branch?

@nik9000
Copy link
Member

nik9000 commented May 15, 2018

@nik9000 Is it ok if I squash these commits in my branch?

Generally we prefer to keep the merge history in PR branches and let github's Squash and merge button do the squashing. Mostly we do this so the review process is easier when the PRs get bigger. Since your PR is tiny it doesn't really matter but we may as well keep the standard practice. So, please do not squash. The merge button will squash for us when I click it.

@elasticmachine, test this please.

@nik9000
Copy link
Member

nik9000 commented May 15, 2018

@nik9000 Let me go ahead and do that.

Also, thanks for fixing it!

@nik9000 nik9000 merged commit f0da3da into elastic:master May 16, 2018
nik9000 pushed a commit that referenced this pull request May 16, 2018
@ashashwat @nik9000
Reindex: Fixed typo in assertion failure message (#30619)
81f29fb 
Fix a typo in an assertion failure message.
@nik9000
Copy link
Member

nik9000 commented May 16, 2018

Thanks again @ashashwat! I've merged your fix to master and backported to 6.x.

martijnvg added a commit that referenced this pull request May 17, 2018
@martijnvg
Merge remote-tracking branch 'es/master' into ccr
5298237 
* es/master: (74 commits)
  Preserve REST client auth despite 401 response (#30558)
  [test] packaging: add windows boxes (#30402)
  Make xpack modules instead of a meta plugin (#30589)
  Mute ShrinkIndexIT
  [ML] DeleteExpiredDataAction should use client with origin (#30646)
  Reindex: Fixed typo in assertion failure message (#30619)
  [DOCS] Fixes list of unconverted snippets in build.gradle
  [DOCS] Reorganizes RBAC documentation
  SQL: Remove dependency for server's version from JDBC driver (#30631)
  Test: increase search logging for LicensingTests
  Adjust serialization version in IndicesOptions
  [TEST] Fix compilation
  Remove version argument in RangeFieldType (#30411)
  Remove unused DirectoryUtils class. (#30582)
  Mitigate date histogram slowdowns with non-fixed timezones. (#30534)
  Add a MovingFunction pipeline aggregation, deprecate MovingAvg agg (#29594)
  Removes AwaitsFix on IndicesOptionsTests
  Template upgrades should happen in a system context (#30621)
  Fix bug in BucketMetrics path traversal (#30632)
  Fixes IndiceOptionsTests to serialise correctly (#30644)
  ...
martijnvg added a commit that referenced this pull request May 17, 2018
@martijnvg
Merge remote-tracking branch 'es/6.x' into ccr-6.x
13f1200 
* es/6.x: (44 commits)
  SQL: Remove dependency for server's version from JDBC driver (#30631)
  Make xpack modules instead of a meta plugin (#30589)
  Security: Remove SecurityLifecycleService (#30526)
  Build: Add task interdependencies for ssl configuration (#30633)
  Mute ShrinkIndexIT
  [ML] DeleteExpiredDataAction should use client with origin (#30646)
  Reindex: Fixed typo in assertion failure message (#30619)
  [DOCS] Fixes list of unconverted snippets in build.gradle
  Use readFully() to read bytes from CipherInputStream (#30640)
  Add Create Repository High Level REST API (#30501)
  [DOCS] Reorganizes RBAC documentation
  Test: increase search logging for LicensingTests
  Delay _uid field data deprecation warning (#30651)
  Deprecate Empty Templates (#30194)
  Remove unused DirectoryUtils class. (#30582)
  Mitigate date histogram slowdowns with non-fixed timezones. (#30534)
  [TEST] Remove AwaitsFix in IndicesOptionsTests#testSerialization
  S3 repo plugin populates SettingsFilter (#30652)
  Rest High Level client: Add List Tasks (#29546)
  Fixes IndiceOptionsTests to serialise correctly (#30644)
  ...
martijnvg added a commit to martijnvg/elasticsearch that referenced this pull request May 17, 2018
@martijnvg
Merge remote-tracking branch 'es/ccr' into ccr_create_and_follow
48d2fc8 
* es/ccr: (75 commits)
  Preserve REST client auth despite 401 response (elastic#30558)
  [test] packaging: add windows boxes (elastic#30402)
  Make xpack modules instead of a meta plugin (elastic#30589)
  Mute ShrinkIndexIT
  [ML] DeleteExpiredDataAction should use client with origin (elastic#30646)
  Reindex: Fixed typo in assertion failure message (elastic#30619)
  [DOCS] Fixes list of unconverted snippets in build.gradle
  [DOCS] Reorganizes RBAC documentation
  SQL: Remove dependency for server's version from JDBC driver (elastic#30631)
  Test: increase search logging for LicensingTests
  Adjust serialization version in IndicesOptions
  [TEST] Fix compilation
  Remove version argument in RangeFieldType (elastic#30411)
  Remove unused DirectoryUtils class. (elastic#30582)
  Mitigate date histogram slowdowns with non-fixed timezones. (elastic#30534)
  Add a MovingFunction pipeline aggregation, deprecate MovingAvg agg (elastic#29594)
  Removes AwaitsFix on IndicesOptionsTests
  Template upgrades should happen in a system context (elastic#30621)
  Fix bug in BucketMetrics path traversal (elastic#30632)
  Fixes IndiceOptionsTests to serialise correctly (elastic#30644)
  ...
ywelsch pushed a commit to ywelsch/elasticsearch that referenced this pull request May 23, 2018
@ashashwat @ywelsch
Reindex: Fixed typo in assertion failure message (elastic#30619)
2decae7 
Fix a typo in an assertion failure message.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
:Distributed Indexing/CRUD A catch all label for issues around indexing, updating and getting a doc by id. Not search. >non-issue v6.4.0 v7.0.0-beta1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ashashwat @elasticmachine @nik9000 @colings86