magibney commented on PR #2199: URL: https://github.com/apache/solr/pull/2199#issuecomment-1894015934
Should we instead have this default at the lowest level to being randomized, or otherwise ensure that nothing (either now or future) falls through the cracks? This would yield a much smaller diff, but more importantly: having the randomized/configurable default at a lower level would improve confidence that we're achieving the goal of test coverage for both PRS and non-PRS case. Ideally it'd be nice to only explicitly specify `CollectionAdminRequest.setPerReplicaState(boolean)` when a test is actually testing behavior that relies specifically on PRS or non-PRS. Specifically, I'm wondering about changing [this line](https://github.com/apache/solr/blob/b5db179164919df657845f5e71f9fe0f0a1c86d8/solr/solrj/src/java/org/apache/solr/client/solrj/request/CollectionAdminRequest.java#L687-L689): ```diff --- a/solr/solrj/src/java/org/apache/solr/client/solrj/request/CollectionAdminRequest.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/request/CollectionAdminRequest.java @@ -684,7 +684,7 @@ public abstract class CollectionAdminRequest<T extends CollectionAdminResponse> addProperties(params, properties); } numReplicas.writeProps(params); - if (Boolean.TRUE.equals(perReplicaState)) { + if (perReplicaState == null ? DEFAULT_PRS : Boolean.TRUE.equals(perReplicaState)) { params.set(CollectionAdminParams.PER_REPLICA_STATE, perReplicaState); } params.setNonNull(ALIAS, alias); ``` ... where `DEFAULT_PRS` would be a new boolean constant (or pseudo-constant) initialized via sysprop (analogous to current `SolrCloudTestCase.USE_PER_REPLICA_STATE`). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
