Skip to content

Conversation

@nik9000
Copy link
Member

@nik9000 nik9000 commented Aug 21, 2018

In #29623 we added Request object flavored requests to the low level
REST client and in #30315 we deprecated the old performRequests. This
changes all calls in the x-pack/qa/smoke-test-monitoring-with-watcher,
x-pack/qa/smoke-test-watcher, and
x-pack/qa/smoke-test-watcher-with-security projects to use the new
versions.

In elastic#29623 we added `Request` object flavored requests to the low level
REST client and in elastic#30315 we deprecated the old `performRequest`s. This
changes all calls in the `x-pack/qa/smoke-test-monitoring-with-watcher`,
`x-pack/qa/smoke-test-watcher`, and
`x-pack/qa/smoke-test-watcher-with-security` projects to use the new
versions.
@nik9000 nik9000 added >non-issue :Clients/Java Low Level REST Client Minimal dependencies Java Client for Elasticsearch v7.0.0 v6.5.0 labels Aug 21, 2018
@nik9000 nik9000 requested a review from javanna August 21, 2018 22:41
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

@nik9000 nik9000 added the review label Aug 21, 2018
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 couple of questions, LGTM otherwise

getWatchHistoryEntry(watchId);

Response response = adminClient().performRequest("GET", "my_test_index/doc/some-id",
Collections.singletonMap("ignore", "404"));
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 the ignore param needed?

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 believe that comes for free with HEAD.

Copy link
Member

Choose a reason for hiding this comment

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

oh right!

// create one document in this index, so we can test in the YAML tests, that the index cannot be accessed
Response resp = adminClient().performRequest("PUT", "/index_not_allowed_to_read/doc/1", Collections.emptyMap(),
new StringEntity("{\"foo\":\"bar\"}", ContentType.APPLICATION_JSON));
assertThat(resp.getStatusLine().getStatusCode(), is(201));
Copy link
Member

Choose a reason for hiding this comment

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

why remove this assertion?

StringEntity entity = new StringEntity(Strings.toString(builder), ContentType.APPLICATION_JSON);

Response response = client().performRequest("PUT", "_xpack/watcher/watch/" + watchId, Collections.emptyMap(), entity);
assertOK(response);
Copy link
Member

Choose a reason for hiding this comment

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

did you remove this assertion intentionally?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. It just asserts that we return a 200 or 201. Since we through an exception on non-2xx replies anyway I figured it isn't useful.

Copy link
Member

Choose a reason for hiding this comment

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

makes sense

@nik9000 nik9000 merged commit c3438bc into elastic:master Aug 22, 2018
nik9000 added a commit that referenced this pull request Aug 22, 2018
In #29623 we added `Request` object flavored requests to the low level
REST client and in #30315 we deprecated the old `performRequest`s. This
changes all calls in the `x-pack/qa/smoke-test-monitoring-with-watcher`,
`x-pack/qa/smoke-test-watcher`, and
`x-pack/qa/smoke-test-watcher-with-security` projects to use the new
versions.
martijnvg added a commit that referenced this pull request Aug 24, 2018
* es/master: (62 commits)
  [DOCS] Add docs for Application Privileges (#32635)
  Add versions 5.6.12 and 6.4.1
  Do NOT allow termvectors on nested fields (#32728)
  [Rollup] Return empty response when aggs are missing (#32796)
  [TEST] Add some ACL yaml tests for Rollup (#33035)
  Move non duplicated actions back into xpack core (#32952)
  Test fix - GraphExploreResponseTests should not randomise array elements Closes #33086
  Use `addIfAbsent` instead of checking if an element is contained
  TESTS: Fix Random Fail in MockTcpTransportTests (#33061)
  HLRC: Fix Compile Error From Missing Throws (#33083)
  [DOCS] Remove reload password from docs cf. #32889
  HLRC: Add ML Get Buckets API (#33056)
  Watcher: Improve error messages for CronEvalTool (#32800)
  Search: Support of wildcard on docvalue_fields (#32980)
  Change query field expansion (#33020)
  INGEST: Cleanup Redundant Put Method (#33034)
  SQL: skip uppercasing/lowercasing function tests for AZ locales as well (#32910)
  Fix the default pom file name (#33063)
  Switch ml basic tests to new style Requests (#32483)
  Switch some watcher tests to new style Requests (#33044)
  ...
martijnvg added a commit that referenced this pull request Aug 24, 2018
* es/6.x: (58 commits)
  [DOCS] Add docs for Application Privileges (#32635)
  Add versions 5.6.12 and 6.4.1
  [Rollup] Return empty response when aggs are missing (#32796)
  [TEST] Add some ACL yaml tests for Rollup (#33035)
  Test fix - GraphExploreResponseTests should not randomise array elements Closes #33086
  Use `addIfAbsent` instead of checking if an element is contained
  HLRC: Fix Compile Error From Missing Throws (#33083)
  [DOCS] Remove reload password from docs cf. #32889
  Use a dedicated ConnectionManger for RemoteClusterConnection (#32988)
  Watcher: Improve error messages for CronEvalTool (#32800)
  HLRC: Add ML Get Buckets API (#33056)
  Change query field expansion (#33020)
  Search: Support of wildcard on docvalue_fields (#32980)
  Add beta label to MSI on install Elasticsearch page (#28126)
  SQL: skip uppercasing/lowercasing function tests for AZ locales as well (#32910)
  [DOCS] Drafts Elasticsearch 6.4.0 release notes (#33039)
  Fix the default pom file name (#33063)
  Fix backport of switch ml basic tests to new style Requests (#32483)
  Switch ml basic tests to new style Requests (#32483)
  Switch some watcher tests to new style Requests (#33044)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Clients/Java Low Level REST Client Minimal dependencies Java Client for Elasticsearch >non-issue v6.5.0 v7.0.0-beta1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants