Skip to content

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

Merged
astefan merged 9 commits intoelastic:masterfrom
astefan:30009fix
Oct 25, 2018
Merged

SQL: handle X-Pack or X-Pack SQL not being available in a more graceful way#34736
astefan merged 9 commits intoelastic:masterfrom
astefan:30009fix

Conversation

@astefan
Copy link
Copy Markdown
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
Copy Markdown
Collaborator

Pinging @elastic/es-search-aggs

Copy link
Copy Markdown
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
Copy Markdown
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

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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Contributor Author

astefan commented Oct 24, 2018

Retest this please.

1 similar comment
@astefan
Copy link
Copy Markdown
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
Copy Markdown
Contributor Author

astefan commented Oct 24, 2018

Retest this please.

1 similar comment
@astefan
Copy link
Copy Markdown
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