Copilot commented on code in PR #4739: URL: https://github.com/apache/solr/pull/4739#discussion_r4194831796
########## solr/core/src/java/org/apache/solr/cli/Deploy.java: ########## @@ -0,0 +1,100 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.solr.cli; + +import static org.apache.solr.cli.SolrCLI.printRed; + +/** Supports package deploy command in the bin/solr script. */ +@SuppressWarnings("UnnecessarilyFullyQualified") [email protected]( + name = "deploy", + description = "Deploy an installed package to collections or at cluster level.", + exitCodeListHeading = "%nExit Codes:%n", + exitCodeList = { + "0: Operation completed successfully.", + "1: Operation failed; check output for details." + }, + footerHeading = "%nExamples:%n", + footer = { + " # Deploy a package to a collection", + " bin/solr package deploy mypkg:1.0.0 --collections myCollection -y", + "", + " # Update an existing deployment", + " bin/solr package deploy mypkg --update --collections myCollection -y" + }) +public class Deploy extends PackageSubCommand { + + @picocli.CommandLine.Parameters( + index = "0", + arity = "1", + paramLabel = "PACKAGE[:VERSION]", + description = "Package name, optionally with :version.") + private String packageNameAndVersion; + + @picocli.CommandLine.Option( + names = {"--cluster"}, + description = "Specifies that this action should affect cluster-level plugins only.") + private boolean cluster; + + @picocli.CommandLine.Option( + names = {"--collections"}, + paramLabel = "COLLECTIONS", + description = + "Specifies that this action should affect plugins for the given collections only, excluding cluster level plugins.") + private String collections; + + @picocli.CommandLine.Option( + names = {"-p", "--param"}, + paramLabel = "PARAMS", + description = "List of parameters to be used with deploy command.") Review Comment: The existing --param option uses hasArgs(), allowing multiple assignments after one occurrence. Picocli defaults this String[] option to one value per occurrence, so `package deploy mypkg --collections c --param A=1 B=2 -y` now rejects B=2 as an extra positional argument. Set arity = "1..*" to preserve this syntax alongside repeated --param options, add a parsing regression test, and regenerate the deploy documentation. ########## solr/core/src/java/org/apache/solr/cli/ConnectionOptions.java: ########## @@ -73,4 +76,78 @@ String effectiveSolrUrl() throws IOException { } return solrUrl; } + + static String resolveSolrUrl(ConnectionOptions connectionOptions, String credentials) + throws Exception { + if (connectionOptions != null) { + String solrUrl = connectionOptions.effectiveSolrUrl(); + if (solrUrl != null) { + return CLIUtils.normalizeSolrUrl(solrUrl); + } + String zkHost = connectionOptions.effectiveZkHost(); + if (zkHost != null) { + return CLIUtils.solrUrlFromConnection( + CloudSolrClient.CloudSolrClientConnection.parse(zkHost), credentials); + } + } + + String solrConnectionProp = EnvUtils.getProperty("solr-connection"); + if (solrConnectionProp != null && !solrConnectionProp.isBlank()) { + var connection = CloudSolrClient.CloudSolrClientConnection.parse(solrConnectionProp); + if (connection.isZookeeper()) { + return CLIUtils.solrUrlFromConnection(connection, credentials); + } + return CLIUtils.normalizeSolrUrl(connection.quorumItems().get(0)); + } + + String zkHostProp = EnvUtils.getProperty("zkHost"); + if (zkHostProp != null && !zkHostProp.isBlank()) { + return CLIUtils.solrUrlFromConnection( + CloudSolrClient.CloudSolrClientConnection.parse(zkHostProp), credentials); + } + + String defaultUrl = CLIUtils.getDefaultSolrUrl(); + CLIO.err( + "Neither --solr-connection, --zk-host or --solr-url parameters, nor SOLR_CONNECTION, ZK_HOST env var provided, so assuming solr url is " + + defaultUrl + + "."); + return defaultUrl; + } + + static String resolveZkHost( + ConnectionOptions connectionOptions, String solrUrl, String credentials) throws Exception { + if (connectionOptions != null) { + String zkHost = connectionOptions.effectiveZkHost(); + if (zkHost != null) { + return zkHost; + } + } + + String solrConnectionProp = EnvUtils.getProperty("solr-connection"); + if (solrConnectionProp != null && !solrConnectionProp.isBlank()) { + var connection = CloudSolrClient.CloudSolrClientConnection.parse(solrConnectionProp); + if (connection.isZookeeper()) { + return solrConnectionProp; + } + } + + String zkHostProp = EnvUtils.getProperty("zkHost"); + if (zkHostProp != null && !zkHostProp.isBlank()) { + return zkHostProp; + } Review Comment: When SOLR_CONNECTION points to ZooKeeper cluster A but an explicit --solr-url or HTTP --solr-connection targets cluster B, resolveSolrUrl selects B while this fallback selects A. PackageManager then combines B's HTTP client with A's ZooKeeper client; add-repo can write repository metadata to A while installing the key through B. Skip both environment fallbacks when an HTTP target is selected and discover ZooKeeper from that target instead. Add a regression test with conflicting explicit and environment targets. -- 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]
