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

SQL: handle X-Pack or X-Pack SQL not being available in a more graceful way #34736

Merged
merged 9 commits into from
Oct 25, 2018

Conversation

astefan
Copy link
Contributor

@astefan astefan commented Oct 23, 2018

Fix for #30009.
The same error code is received when either the x-pack is not available at all, or SQL is explicitly disabled (xpack.sql.enabled: false).

@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search-aggs

Copy link
Contributor

@matriv matriv left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@costin costin left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -176,6 +176,16 @@ private boolean shouldParseBody(int responseCode) {
}
SqlExceptionType type = SqlExceptionType.fromRemoteFailureType(failure.type());
if (type == null) {
if (con.getResponseCode() == HttpURLConnection.HTTP_BAD_REQUEST) {
return new ResponseOrException<>(new SQLException("It doesn't look like the X-Pack or the X-Pack SQL component"
Copy link
Member

Choose a reason for hiding this comment

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

Slight rephrasing, removing some the and typos: X-Pack/ SQL do not seem to be available on the Elasticsearch node using the access path '...' . Please verify X-Pack is installed and SQL enabled. Alternatively, check if any proxy is interfering the communication to Elasticsearch.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. I modified it.

@astefan
Copy link
Contributor Author

astefan commented Oct 24, 2018

@costin @matriv sorry to resurrect this one, but the 400 code is returned in other situations as well (looking into a different bug showed the other scenario). I changed the way we check for x-pack/sql being available by comparing the error message itself. Not elegant, but the alternative is probably an additional request being made for every sql request going to ES.

@astefan
Copy link
Contributor Author

astefan commented Oct 24, 2018

Retest this please.

1 similar comment
@astefan
Copy link
Contributor Author

astefan commented Oct 24, 2018

Retest this please.

@astefan astefan changed the title SQL: handle X-Pack or X-Pack SQL not being available in a more graceful way in the JDBC driver SQL: handle X-Pack or X-Pack SQL not being available in a more graceful way Oct 24, 2018
@astefan
Copy link
Contributor Author

astefan commented Oct 24, 2018

Retest this please.

1 similar comment
@astefan
Copy link
Contributor Author

astefan commented Oct 25, 2018

Retest this please.

@astefan astefan merged commit a7e08f4 into elastic:master Oct 25, 2018
@astefan astefan deleted the 30009fix branch October 25, 2018 09:15
astefan added a commit that referenced this pull request Oct 25, 2018
…ul way (#34736)

Throw a different error message for a http response code of 400, but also when the error itself is of a specific type.
astefan added a commit to astefan/elasticsearch that referenced this pull request Oct 25, 2018
…ul way (elastic#34736)

Throw a different error message for a http response code of 400, but also when the error itself is of a specific type.
astefan added a commit that referenced this pull request Oct 25, 2018
…ul way (#34736)

Throw a different error message for a http response code of 400, but also when the error itself is of a specific type.
@astefan astefan added the v6.6.0 label Oct 25, 2018
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Oct 26, 2018
* master: (74 commits)
  XContent: Check for bad parsers (elastic#34561)
  Docs: Align prose with snippet (elastic#34839)
  document the search context is freed if the scroll is not extended (elastic#34739)
  Test: Lookup node versions on rest test start (elastic#34657)
  SQL: Return error with ORDER BY on non-grouped. (elastic#34855)
  Reduce channels in AbstractSimpleTransportTestCase (elastic#34863)
  [DOCS] Updates Elasticsearch monitoring tasks (elastic#34339)
  Check self references in metric agg after last doc collection (elastic#33593) (elastic#34001)
  [Docs] Add `indices.query.bool.max_clause_count` setting (elastic#34779)
  Add 6.6.0 version to master (elastic#34847)
  Test: ensure char[] doesn't being with prefix (elastic#34816)
  Remove static import from HLRC doc snippet (elastic#34834)
  Logging: server: clean up logging (elastic#34593)
  Logging: tests: clean up logging (elastic#34606)
  SQL: Fix edge case: `<field> IN (null)` (elastic#34802)
  [Test] Mute FullClusterRestartIT.testShrink() until test is fixed
  SQL: Introduce ODBC mode, similar to JDBC (elastic#34825)
  SQL: handle X-Pack or X-Pack SQL not being available in a more graceful way (elastic#34736)
  [Docs] Add explanation for code snippets line width (elastic#34796)
  CCR: Rename follow-task parameters and stats (elastic#34836)
  ...
kcm pushed a commit that referenced this pull request Oct 30, 2018
…ul way (#34736)

Throw a different error message for a http response code of 400, but also when the error itself is of a specific type.
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.

5 participants