SQL: Remove the last remaining server dependencies from jdbc #30771
Conversation
Removes the last remaining server dependencies from the shared proto classes and http client.
|
Pinging @elastic/es-search-aggs |
The
We should definitely take a look at it as a follow-up. I think the CLI change is relatively simple, but my attempts to resolve for the security dependency were causing circular dependencies that I wasn't quite sure how to resolve. The biggest problem here is that SQL depends on security.... and security tests would need to depend on SQL. The only other path that I still need to explore is moving these tests from security to SQL, but that might require quite a bit of code duplication. Another path worth exploring is to merge |
Makes sense to me! |
| compile (project(':libs:x-content')) { | ||
| transitive = false | ||
| } | ||
| compile "org.apache.lucene:lucene-core:${versions.lucene}" | ||
| compile 'joda-time:joda-time:2.9.9' | ||
| compile project(':libs:elasticsearch-core') | ||
| runtime "com.fasterxml.jackson.core:jackson-core:${versions.jackson}" |
There was a problem hiding this comment.
We'll want ot see about blasting these too in a followup. I think just removing the server dep is good enough for one PR.
| ignoreSha 'elasticsearch-core' | ||
| } | ||
|
|
||
| //thirdPartyAudit.excludes = [ |
| Object value = null; | ||
| if (token == XContentParser.Token.VALUE_STRING) { | ||
| //binary values will be parsed back and returned as base64 strings when reading from json and yaml | ||
| value = parser.text(); |
There was a problem hiding this comment.
Or is this copied from somewhere?
There was a problem hiding this comment.
Ah, yes, XContentParserUtils.
I'm fine with creating this for now, but maybe you can move this method to :libs;x-content in a followup? That'd be awesome. But tricky because the exception type changes. It'd probably be ok because we've made sure that the new exception type reports well though.
There was a problem hiding this comment.
I was looking into it and discussed it with @dakrone. It's tricky. I had to modify the it to get it here. The one in XContentParselUtils is also handling binary data that it converts into lucene's BytesArray and throws an exception that builds on the top of server's ElasticsearchException hierarchy. Pulling it out would be quite complicated without major refactoring of all existing users.
There was a problem hiding this comment.
I think doing what you've got is fine. Could you open up an issue to discuss removing the duplication?
…ch-dependency-from-jdbc-v2
…ch-dependency-from-jdbc-v2
…ch-dependency-from-jdbc-v2
…ch-dependency-from-jdbc-v2
…ch-dependency-from-jdbc-v2
Removes the last remaining server dependencies from jdbc client. In order to do that it introduces the new project sql-shared-proto that contains only XContent-serializable classes. HTTP Client and JDBC now depend only on sql-shared-proto. I had to keep the original sql-proto project since it is used as a dependency by sql-cli and security integration tests. Relates #29856
* master: silence InstallPluginCommandTests, see #30900 Remove left-over comment Fix double semicolon in import statement [TEST] Fix minor random bug from #30794 Include size of snapshot in snapshot metadata #18543, bwc clean up (#30890) Enabling testing against an external cluster (#30885) Add public key header/footer (#30877) SQL: Remove the last remaining server dependencies from jdbc (#30771) Include size of snapshot in snapshot metadata (#29602) Do not serialize basic license exp in x-pack info (#30848) Change BWC version for VerifyRepositoryResponse (#30796) [DOCS] Document index name limitations (#30826) Harmonize include_defaults tests (#30700)
* 6.x: Fix double semicolon in import statement [TEST] Fix minor random bug from #30794 Enabling testing against an external cluster (#30885) SQL: Remove the last remaining server dependencies from jdbc (#30771) Add public key header/footer (#30877) Include size of snapshot in snapshot metadata (#29602) QA: Test template creation during rolling restart (#30850) REST high-level client: add put ingest pipeline API (#30793) Do not serialize basic license exp in x-pack info (#30848) [docs] explainer for java packaging tests (#30825) Verify signatures on official plugins (#30800) [DOCS] Document index name limitations (#30826) [Docs] Add reindex.remote.whitelist example (#30828)
Removes the last remaining server dependencies from jdbc client. In order to do that it introduces the new project sql-shared-proto that contains only XContent-serializable classes. HTTP Client and JDBC now depend only on sql-shared-proto. I had to keep the original sql-proto project since it is used as a dependency by sql-cli and security integration tests.
Relates #29856