[jira] [Commented] (SOLR-6736) A collections-like request handler to manage solr configurations on zookeeper

["Gregory Chanan (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/SOLR-6736?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15004844#comment-15004844 ] 

Gregory Chanan commented on SOLR-6736:
--------------------------------------

The approach of doing the upload directly in the handler seems problematic because it doesn't involve the exclusivity enforcement in the Overseer.  Are you sure if you do things like concurrently UPLOAD and DELETE things will be left in a sensible state?  I guess the alternative is writing the zip into the zookeeper message (could that overrun the jute.maxbuffer) or uploading it someone else in ZK and having the Overseer copy it to the correct location.  The later means you have to upload twice, but I'm guessing this operation is rare anyway.

I also don't see any signature checking for security.  I don't see how we can have this feature without security.

More minor comments:
{code}
+      if (action == ConfigSetAction.UPLOAD) {
+        if (isUploadEnabled) {
+          hanldeConfigUploadRequest(req, rsp);
+          return;
+        } else {
+          throw new SolrException(SolrException.ErrorCode.UNAUTHORIZED, 
+              "Uploads are not enabled. Please set the system property \"" 
+                  + ConfigSetParams.ENABLE_CONFIGSET_UPLOAD + "\" to true");
+        }
+      }
{code}
I don't see why this can't follow the usual operation.call path.  Also handle is not spelled correctly.

{code}
+    String httpMethod = (String) req.getContext().get("httpMethod");
+    if (!"POST".equals(httpMethod)) {
+      throw new SolrException(ErrorCode.BAD_REQUEST,
+          "The upload action supports POST requests only");
+    }
{code}
If we are going to check this stuff, I'd rather enforce it for all ConfigSet requests.  This can be done by storing the allowed verbs in the ConfigSetAction enum itself or somewhere in the handler.

{code}
+    if (zkClient.exists(configPathInZk, true)) {
+      throw new SolrException(ErrorCode.SERVER_ERROR,
+          "The configuration " + configSetName + " already exists in zookeeper");
+    }
+    
{code}
This looks like it should be a BAD_REQUEST?

{code}
+    for (ContentStream contentStream : req.getContentStreams()) {
      ..
+      break;
+    }
{code}
This just reads the first content stream?  Why have a loop then?

How about solrj classes for the requests/responses that you can use in the test, instead of hand parsing everything?

{code}
+    //Checking error when mo configuration name is specified in request
{code}
mo -> no

> A collections-like request handler to manage solr configurations on zookeeper
> -----------------------------------------------------------------------------
>
>                 Key: SOLR-6736
>                 URL: https://issues.apache.org/jira/browse/SOLR-6736
>             Project: Solr
>          Issue Type: New Feature
>          Components: SolrCloud
>            Reporter: Varun Rajput
>            Assignee: Anshum Gupta
>         Attachments: SOLR-6736-newapi.patch, SOLR-6736-newapi.patch, SOLR-6736.patch, SOLR-6736.patch, SOLR-6736.patch, SOLR-6736.patch, SOLR-6736.patch, SOLR-6736.patch, SOLR-6736.patch, SOLR-6736.patch, newzkconf.zip, test_private.pem, test_pub.der, zkconfighandler.zip, zkconfighandler.zip
>
>
> Managing Solr configuration files on zookeeper becomes cumbersome while using solr in cloud mode, especially while trying out changes in the configurations. 
> It will be great if there is a request handler that can provide an API to manage the configurations similar to the collections handler that would allow actions like uploading new configurations, linking them to a collection, deleting configurations, etc.
> example : 
> {code}
> #use the following command to upload a new configset called mynewconf. This will fail if there is alredy a conf called 'mynewconf'. The file could be a jar , zip or a tar file which contains all the files for the this conf.
> curl -X POST -H 'Content-Type: application/octet-stream' --data-binary @testconf.zip http://localhost:8983/solr/admin/configs/mynewconf?sig=<the-signature>
> {code}
> A GET to http://localhost:8983/solr/admin/configs will give a list of configs available
> A GET to http://localhost:8983/solr/admin/configs/mynewconf would give the list of files in mynewconf



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org