dsmiley commented on code in PR #4672:
URL: https://github.com/apache/solr/pull/4672#discussion_r3657224184
##########
solr/core/src/java/org/apache/solr/cli/CreateTool.java:
##########
@@ -226,53 +206,37 @@ protected void createCollection(CloudSolrClient
cloudSolrClient, CommandLine cli
ensureConfDirExists(solrInstallDirPath, confDirPath);
printDefaultConfigsetWarningIfNecessary(cli);
- Set<String> liveNodes = cloudSolrClient.getClusterState().getLiveNodes();
- if (liveNodes.isEmpty())
- throw new IllegalStateException(
- "No live nodes found! Cannot create a collection until "
- + "there is at least 1 live node in the cluster.");
-
- String solrUrl;
- if (CLIUtils.hasConnectionOption(cli)) {
- solrUrl = CLIUtils.normalizeSolrUrl(cli);
- } else {
- String firstLiveNode = liveNodes.iterator().next();
- solrUrl =
ZkStateReader.from(cloudSolrClient).getBaseUrlForNodeName(firstLiveNode);
- }
+ String solrUrl = CLIUtils.normalizeSolrUrl(cli);
// build a URL to create the collection
int numShards = cli.getParsedOptionValue(SHARDS_OPTION, 1);
int replicationFactor =
cli.getParsedOptionValue(REPLICATION_FACTOR_OPTION, 1);
- boolean configExistsInZk =
+ boolean configExists =
confName != null
&& !confName.trim().isEmpty()
- &&
ZkStateReader.from(cloudSolrClient).getZkClient().exists("/configs/" +
confName);
+ && new
ConfigsetsApi.ListConfigSet().process(solrClient).configSets.contains(confName);
- if (configExistsInZk) {
+ if (configExists) {
echo("Re-using existing configuration directory " + confName);
} else { // if (confdir != null && !confdir.trim().isEmpty()) {
if (confName == null || confName.trim().isEmpty()) {
confName = collectionName;
}
- // TODO: This should be done using the configSet API. This would let us
remove
- // the direct dependency on ZooKeeper APIs. Unlike the bin/solr zk
comamnds that
- // work directly with ZooKeeper.
final Path configsetsDirPath =
CLIUtils.getConfigSetsDir(solrInstallDirPath);
- ConfigSetService configSetService =
- new
ZkConfigSetService(ZkStateReader.from(cloudSolrClient).getZkClient());
Path confPath = ConfigSetService.getConfigsetPath(confDir,
configsetsDirPath.toString());
echoIfVerbose(
"Uploading "
+ confPath.toAbsolutePath()
+ " for config "
+ confName
- + " to ZooKeeper at "
- + cloudSolrClient.getClusterStateProvider().getQuorumHosts());
- // We will trust the config since we have the Zookeeper Address
- configSetService.uploadConfig(confName, confPath);
+ + " using the Configsets V2 API");
Review Comment:
must we speak of the fact that we use that API specifically, especially
"V2"?
##########
solr/core/src/java/org/apache/solr/cli/CreateTool.java:
##########
@@ -314,6 +278,26 @@ protected void createCollection(CloudSolrClient
cloudSolrClient, CommandLine cli
echo(endMessage);
}
+ /** Zips the contents of a configset directory for upload via the Configsets
V2 API. */
Review Comment:
@mlbiscoc I know you wrote this code as well at work so, yeah, a reusable
method would be great.
Eric; what's the issue with zero-byte file?
##########
solr/core/src/java/org/apache/solr/cli/CreateTool.java:
##########
@@ -314,6 +278,26 @@ protected void createCollection(CloudSolrClient
cloudSolrClient, CommandLine cli
echo(endMessage);
}
+ /** Zips the contents of a configset directory for upload via the Configsets
V2 API. */
Review Comment:
there is nothing V2 specific about this or Solr API specific; should not
speak of such.
+1 to a helper somewhere... hopefully discoverable/findable in an expected
spot.
##########
solr/core/src/java/org/apache/solr/cli/CreateTool.java:
##########
@@ -125,7 +127,7 @@ public Options getOptions() {
public void runImpl(CommandLine cli) throws Exception {
try (var solrClient = CLIUtils.getSolrClient(cli)) {
Review Comment:
You could use `CLIUtils.getCloudSolrClient(zkSolrConnection, builder)`
here...
##########
solr/core/src/java/org/apache/solr/cli/CreateTool.java:
##########
@@ -125,7 +127,7 @@ public Options getOptions() {
public void runImpl(CommandLine cli) throws Exception {
try (var solrClient = CLIUtils.getSolrClient(cli)) {
if (CLIUtils.isCloudMode(solrClient)) {
- createCollection(cli);
+ createCollection(cli, solrClient);
} else {
createCore(cli, solrClient);
Review Comment:
... and if the mode isn't actually cloud then at this line call
cloudSolrClient.getHttpSolrClient().
--
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]
