SOLR-16391: Convert create-core, core-status, /luke to JAX-RS #3054
Conversation
Still need to run tests and do a final once-over of the code, but this appears to be pretty close.
This is not working, or even compiling, but I wanted to checkpoint my work to share it between machines. When resuming, look for the TODO comment in StatusOp.java.
Builds on the previous commit's broken state and brings things to a point where they should at least compile so that I can run tests and start catching bugs. TODO/NOCOMMITs throughout the changes still, and there are a few tests I know fail from the earlier create-core change, but we can start to catch things.
|
Still TODO: expand response POJOs usage in tests where appropriate, ref-guide updates for APIs, flag methods-needing-deprecation with PR comments. |
|
As my 13 year old would say, you are cookin! Out of curiosity, at this point, do you think we need to keep up the emphasis on back porting to 9x.. If we smell 10 on the horizon, maybe we don't worry about backporting...... |
| final var coreStatusReq = new CoresApi.GetCoreStatus(coreName); | ||
| final var coreStatusRsp = coreStatusReq.process(solrClient).getParsed(); | ||
| final var coreStatusByName = coreStatusRsp.status; | ||
| final var coreStatus = coreStatusByName.get(coreName); |
There was a problem hiding this comment.
these var are a lot more readable...
| final var initFailureForCore = failureStatus.get(coreName); | ||
| final boolean hasName = coreStatus != null && coreStatus.name != null; | ||
| exists = hasName || initFailureForCore != null; | ||
| wait = hasName && initFailureForCore == null && "true".equals(coreStatus.isLoading); |
There was a problem hiding this comment.
boo that we are string comparing versus using a boolean...
There was a problem hiding this comment.
I feel like I see boolean true used elsewhere for isLoading?
| @@ -92,32 +82,12 @@ public enum CoreAdminOperation implements CoreAdminOp { | |||
| CREATE_OP( | |||
| CREATE, | |||
| it -> { | |||
| assert TestInjection.injectRandomDelayInCoreCreation(); | |||
There was a problem hiding this comment.
take it we don't need this in the future? Or a pattern we don't want to continue?
There was a problem hiding this comment.
Eric found this later on his own, but for anyone else, this line was moved but not removed. See here
| info.add("schema", core.getSchemaResource()); | ||
| info.add("startTime", core.getStartTimeStamp()); | ||
| info.add("uptime", core.getUptimeMs()); | ||
| info.name = core.getName(); |
| } | ||
|
|
||
| private CreateCoreResponse doCreate(CreateCoreParams requestBody, CreateCoreResponse response) { | ||
| assert TestInjection.injectRandomDelayInCoreCreation(); |
There was a problem hiding this comment.
Huh.. Okay, maybe thsi line explains why the other had it removed!
Thought this code was already on the branch prior to creating the PR, but neglected to commit/push I guess. Tests should be passing with this commit.
https://issues.apache.org/jira/browse/SOLR-16391
Description
Many v2 APIs remain on the legacy v2 API framework, including a number of APIs dealing with core CRUD and status-checking.
Solution
This commit migrates four APIs: create-core, single-core-status, all-core-status, and the "luke" request handler over to the JAX-RS framework. This adds the APIs to our growing OpenAPI spec, and ensures that SolrRequest/SolrResponse implementations are generated for each.
Since the v1 and v2 response formats are the same for each API converted in this PR, the response POJOs are also useful in existing tests for our v1 APIs, and this PR rolls out some of that usage to clear up needless casting and NamedList inspection.
Lastly, this PR modifies some 'public'-visibility SolrJ methods that will need properly deprecated in an eventual 9x backport. I've tried to flag those with a PR comment where I'm aware of the change. Please point out any others I've missed.
Tests
TBD
Checklist
Please review the following and check all that apply:
mainbranch../gradlew check.