Re: [PR] SOLR-14496: Add Basic Auth support to SolrCLI [solr]

2023-10-17 Thread via GitHub
epugh merged PR #1954: URL: https://github.com/apache/solr/pull/1954 -- 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: issues-unsubscr...@solr.apache.org

Re: [PR] SOLR-14496: Add Basic Auth support to SolrCLI [solr]

2023-10-13 Thread via GitHub
epugh commented on PR #1954: URL: https://github.com/apache/solr/pull/1954#issuecomment-1762152465 I'd like to merge this next week, can I get another LGTM on this code? Any other concerns? -- This is an automated message from the Apache Git Service. To respond to the message, please log

Re: [PR] SOLR-14496: Add Basic Auth support to SolrCLI [solr]

2023-10-03 Thread via GitHub
epugh commented on PR #1954: URL: https://github.com/apache/solr/pull/1954#issuecomment-1746032490 ``` 2> SimplePostTool: FATAL: Connection error (is Solr running at https://127.0.0.1:54967/solr/aliasedCollection ?): javax.net.ssl.SSLHandshakeException: PKIX path building failed: sun.se

Re: [PR] SOLR-14496: Add Basic Auth support to SolrCLI [solr]

2023-10-03 Thread via GitHub
epugh commented on PR #1954: URL: https://github.com/apache/solr/pull/1954#issuecomment-1746031146 > are the 2 test failures actual problems? > > ``` > - org.apache.solr.cli.PostToolTest.testBasicRun (:solr:core) > Test output: /tmp/src/solr/solr/core/build/test-results/te

Re: [PR] SOLR-14496: Add Basic Auth support to SolrCLI [solr]

2023-10-03 Thread via GitHub
stillalex commented on PR #1954: URL: https://github.com/apache/solr/pull/1954#issuecomment-1746016444 are the 2 test failures actual problems? ``` - org.apache.solr.cli.PostToolTest.testBasicRun (:solr:core) Test output: /tmp/src/solr/solr/core/build/test-results/test/outputs

Re: [PR] SOLR-14496: Add Basic Auth support to SolrCLI [solr]

2023-10-03 Thread via GitHub
epugh commented on code in PR #1954: URL: https://github.com/apache/solr/pull/1954#discussion_r1344827605 ## solr/solr-ref-guide/modules/deployment-guide/pages/solr-control-script-reference.adoc: ## @@ -577,6 +578,16 @@ This parameter is unnecessary if `ZK_HOST` is defined in `

Re: [PR] SOLR-14496: Add Basic Auth support to SolrCLI [solr]

2023-10-03 Thread via GitHub
stillalex commented on code in PR #1954: URL: https://github.com/apache/solr/pull/1954#discussion_r1344807217 ## solr/solr-ref-guide/modules/deployment-guide/pages/solr-control-script-reference.adoc: ## @@ -577,6 +578,16 @@ This parameter is unnecessary if `ZK_HOST` is defined i

Re: [PR] SOLR-14496: Add Basic Auth support to SolrCLI [solr]

2023-10-03 Thread via GitHub
stillalex commented on code in PR #1954: URL: https://github.com/apache/solr/pull/1954#discussion_r1344807719 ## solr/solr-ref-guide/modules/deployment-guide/pages/solr-control-script-reference.adoc: ## @@ -734,6 +745,16 @@ Unnecessary if `ZK_HOST` is defined in `solr.in.sh` or

Re: [PR] SOLR-14496: Add Basic Auth support to SolrCLI [solr]

2023-10-03 Thread via GitHub
stillalex commented on code in PR #1954: URL: https://github.com/apache/solr/pull/1954#discussion_r1344807719 ## solr/solr-ref-guide/modules/deployment-guide/pages/solr-control-script-reference.adoc: ## @@ -734,6 +745,16 @@ Unnecessary if `ZK_HOST` is defined in `solr.in.sh` or

Re: [PR] SOLR-14496: Add Basic Auth support to SolrCLI [solr]

2023-10-03 Thread via GitHub
stillalex commented on code in PR #1954: URL: https://github.com/apache/solr/pull/1954#discussion_r1344807217 ## solr/solr-ref-guide/modules/deployment-guide/pages/solr-control-script-reference.adoc: ## @@ -577,6 +578,16 @@ This parameter is unnecessary if `ZK_HOST` is defined i

Re: [PR] SOLR-14496: Add Basic Auth support to SolrCLI [solr]

2023-10-03 Thread via GitHub
epugh commented on PR #1954: URL: https://github.com/apache/solr/pull/1954#issuecomment-1745449308 Ref Guide is updated! I think I'm ready for final review on this Are there any other tools/cli items that *should* support Basic auth that I missed? -- This is an automated message f

Re: [PR] SOLR-14496: Add Basic Auth support to SolrCLI [solr]

2023-10-02 Thread via GitHub
epugh commented on PR #1954: URL: https://github.com/apache/solr/pull/1954#issuecomment-1743656420 Still need to update the Ref Guide... Hoping to get clarity on if we can re-purpose the `-u` parameter ;-) -- This is an automated message from the Apache Git Service. To respond to the mes

Re: [PR] SOLR-14496: Add Basic Auth support to SolrCLI [solr]

2023-10-02 Thread via GitHub
epugh commented on PR #1954: URL: https://github.com/apache/solr/pull/1954#issuecomment-1743655383 > `Dbasicauth` I looked at `test_ssl.bats` and it has these lines: ``` solr start -c solr auth enable -type basicAuth -credentials name:password solr assert --starte

Re: [PR] SOLR-14496: Add Basic Auth support to SolrCLI [solr]

2023-10-02 Thread via GitHub
epugh commented on PR #1954: URL: https://github.com/apache/solr/pull/1954#issuecomment-1743526141 > Will this still support using `-Dbasicauth` and `-Dsolr.httpclient.builder.factory` in `SOLR_TOOL_OPTS`? It *should*, however, to be honest, I've never used those parameters... Do

Re: [PR] SOLR-14496: Add Basic Auth support to SolrCLI [solr]

2023-10-02 Thread via GitHub
HoustonPutman commented on PR #1954: URL: https://github.com/apache/solr/pull/1954#issuecomment-1743507341 Will this still support using `-Dbasicauth` and `-Dsolr.httpclient.builder.factory` in `SOLR_TOOL_OPTS`? -- This is an automated message from the Apache Git Service. To respond to th

Re: [PR] SOLR-14496: Add Basic Auth support to SolrCLI [solr]

2023-10-02 Thread via GitHub
stillalex commented on code in PR #1954: URL: https://github.com/apache/solr/pull/1954#discussion_r1342889198 ## solr/core/src/java/org/apache/solr/cli/AssertTool.java: ## @@ -123,6 +123,14 @@ public List getOptions() { Option.builder("e") .desc("Return an

Re: [PR] SOLR-14496: Add Basic Auth support to SolrCLI [solr]

2023-10-02 Thread via GitHub
epugh commented on code in PR #1954: URL: https://github.com/apache/solr/pull/1954#discussion_r1342815745 ## solr/core/src/java/org/apache/solr/cli/ExportTool.java: ## @@ -202,7 +209,16 @@ DocsSink getSink() { abstract void exportDocs() throws Exception; void fetchUn

Re: [PR] SOLR-14496: Add Basic Auth support to SolrCLI [solr]

2023-10-02 Thread via GitHub
epugh commented on code in PR #1954: URL: https://github.com/apache/solr/pull/1954#discussion_r1342810570 ## solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java: ## @@ -1311,6 +1311,22 @@ public Builder withProxyConfiguration( this.proxyIsSecure = i

Re: [PR] SOLR-14496: Add Basic Auth support to SolrCLI [solr]

2023-10-02 Thread via GitHub
epugh commented on PR #1954: URL: https://github.com/apache/solr/pull/1954#issuecomment-1743178358 So, one thing I noticed is that the method `withOptionalBasicAuthCredentials` is ONLY on Http2SolrClient.Builder. It isn't on the `HttpSolrClient builder`. Or on some of the other variations

Re: [PR] SOLR-14496: Add Basic Auth support to SolrCLI [solr]

2023-10-02 Thread via GitHub
epugh commented on code in PR #1954: URL: https://github.com/apache/solr/pull/1954#discussion_r1342785865 ## solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java: ## @@ -1311,6 +1311,22 @@ public Builder withProxyConfiguration( this.proxyIsSecure = i

Re: [PR] SOLR-14496: Add Basic Auth support to SolrCLI [solr]

2023-10-02 Thread via GitHub
epugh commented on code in PR #1954: URL: https://github.com/apache/solr/pull/1954#discussion_r1342625748 ## solr/core/src/java/org/apache/solr/cli/AssertTool.java: ## @@ -123,6 +123,14 @@ public List getOptions() { Option.builder("e") .desc("Return an exit

Re: [PR] SOLR-14496: Add Basic Auth support to SolrCLI [solr]

2023-10-02 Thread via GitHub
epugh commented on code in PR #1954: URL: https://github.com/apache/solr/pull/1954#discussion_r1342625290 ## solr/core/src/java/org/apache/solr/cli/ApiTool.java: ## @@ -57,27 +57,28 @@ public List getOptions() { .hasArg() .required(true) .d