Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Are admin collection operations possible? #138

Closed
gjs29 opened this issue Jul 23, 2015 · 17 comments
Closed

Are admin collection operations possible? #138

gjs29 opened this issue Jul 23, 2015 · 17 comments
Assignees
Labels

Comments

@gjs29
Copy link

gjs29 commented Jul 23, 2015

As part of my use case for a Solr client for SolrCloud I'd like to create a collection using the admin collection APIs (https://cwiki.apache.org/confluence/display/solr/Collections+API). For example: /admin/collections?action=CREATE

Is there a way to administer collections using this client? The examples focus on search and indexing, so maybe it's beyond the scope of this client.

Thanks,

  • Greg
@ColadaFF
Copy link
Collaborator

Hi Greg,

Right now, this is beyong the scope of this client.

But you're welcome to submit a Pull request about that feature.

@luketaverne luketaverne self-assigned this Aug 26, 2015
@nicolasembleton
Copy link
Collaborator

@luketaverne Are you working on this?

@nicolasembleton
Copy link
Collaborator

I think it'd be a nice feature to have though. But like @ColadaFF said, it feels outside the scope of this client...

@luketaverne
Copy link
Collaborator

As we speak :). I'll post a commit by the end of the day to see what you
and @lbdremy think before I write too much.
On Sep 1, 2015 11:15 AM, "nicolasembleton" notifications@github.com wrote:

@luketaverne https://github.com/luketaverne Are you working on this?


Reply to this email directly or view it on GitHub
#138 (comment)
.

@nicolasembleton
Copy link
Collaborator

Awesome. Thanks @luketaverne. The Trello card: https://trello.com/c/U1y3Rvf3/3-epic-admin-collection-operations
But I've left it open to breaking it down as it feels like a big story. So if you can refine the scope of what you're working on on this card or a new one.
Thanks.

@luketaverne
Copy link
Collaborator

@nicolasembleton @lbdremy So here is my implementation. I haven't had time to test it yet, so there might be some bugs, but I'd appreciate it if you could take a look at the overall idea of the implementation before I keep working, ie if having the separate collections.js file is a good idea, if the change to solr.js makes sense (it's kind of a band-aid fix, I don't like it), and if I'm going about this sensibly :).

I only added support for the "Create Collection" function thus far. The others are similar to add.

b655e35

@nicolasembleton
Copy link
Collaborator

Very nice.

So the process would be to create a collection, pass params to create() and then ask the client.admin(createCollection, cb).

I'm wondering if it couldn't be the other way around? Passing the client to the collection handler? And then the create() would do the .get() request?

var createCollection = new Collection(); createCollection.create(client, options, callback);

Or if you want to split create() and the actual action, add an collection.execute(client, cb) method that will just proxy the .get() method?

Regardless, you're definitely on to something. The prospective to manage collections from the lib is very exciting.

@luketaverne
Copy link
Collaborator

Yeah that's the process. I thought to follow the process the library currently uses for query. You create a client, create a query, then pass the query to the client's search() method with a callback. It seems like it might easier for users to follow the same workflow for both search() and admin().

I thought about having the create() method call get() automatically, but that prevents the user from being able to add custom parameters via the set() method. Having two separate calls lets them add the custom params before executing the collection.

I'm not sure whether collection.execute() or client.admin() is a better idea... If I added collection.execute() as a proxy for the .get() method, then isn't it a problem to reference a function in another js file? I'd have to require the client.js functions inside collection.js?

Whatever we decide, I'll keep working on brushing it up and adding main features. Thanks for the feedback!

@luketaverne
Copy link
Collaborator

Also, after some testing, it appears this API that I'm working from only works in SolrCloud mode. If one wishes to edit cores in standalone mode, one must use this API.

I incorrectly assumed that 'collections' == 'cores'. Would there be interest in having a similar set of functions for cores in addition to collections?

@luketaverne
Copy link
Collaborator

Latest commit here: c78d505.

Added all features from the Solr 5 Collections Wiki.

The workflow is now as follows:

var solr = require('solr-client');
var client = solr.createClient({
  solrVersion: 5.0
});

//create the collection object. Name based on client.createQuery().
var collection = client.createCollection();

//collection.create() tells Solr to create a new collection.
collection.create({name:"solrtest2",numShards:1});

//execute the update. I chose client.executeCollection() so we have
//room for client.executeCore(), should we choose to add it in the future.
client.executeCollection(collection, function(err,obj) {
  if(err){
    console.log(err);
  } else {
    console.log(obj);
  }
});

I'm a little dissatisfied with the naming scheme of client.createCollection() and collection.create(). I'm concerned that people will think that client.createCollection() is responsible for telling Solr to create a new collection, when it actually creates a new Collection object in node. Open to suggestions on this topic.

There are a lot of functions in this commit. If someone wants to help me test them I'd really appreciate it. Otherwise, @nicolasembleton and/or @lbdremy, let me know when it's okay to merge these changes to master.

@nicolasembleton
Copy link
Collaborator

Very nice work. I was trying to find all the commits related to this area but couldn't find it for some reason? Is it in a branch or on your fork?

Regardless, very nice job... Reading through your example, everything makes sense, but I agree with your point that there might be confusion. Maybe we could introduce the 'Manager' keyword? 'createCollectionManager()' or something like that? It's a bit heavy, but tells what it is. The word 'Manager' is just an example, there might be better term for that? 'Admin' (but... hides other meanings), 'Proxy' (not quite), etc...

Regarding your previous comment about client.executeCollection or collection.execute(client), I totally understand. We should probably consider a cleanup by 1.X? But currently it goes within the library logic. So I think that's oki.

With regards to testing, do we want to add tests for the updates you made? That would require some setup though? I'm in the process of adding the ability to run the tests through a Docker so you don't have to mess up with any local running copy of your Solr. Maybe we could do the same for SolrCloud? That'd be quite exciting.

I'll be getting your code (plz tell me how :), cf the first paragraph) and run tests on my end. I think it looks very good so far for a first iteration.

@luketaverne
Copy link
Collaborator

It's a branch called 'admin-collections' on my fork. I hesitate to create anything directly on the main project for fear that I'll mess something up :).

Maybe we just use client.collection(), and then in the cleanup we change the query one to client.query() as well? Or just add that right now, so users can start to switch, then in the 1.0 release we can remove the client.createQuery()? Then we have room for client.core() and others.

I think tests are a good idea. Running something with Docker would be nice too. Let me know if I can help with that part. I've never used this test writing language (mocha?), but I'll give it a try on my own machine for now.

Thanks for reading!

@nicolasembleton
Copy link
Collaborator

Maybe we just use client.collection(), and then in the cleanup we change the query one to client.query() as well? Or just add that right now, so users can start to switch, then in the 1.0 release we can remove the client.createQuery()? Then we have room for client.core() and others.

That sounds like a good plan. We should start planning for 0.6 and include those things. And add the mention somewhere that we'll refactor the naming. I think as long as we inform a few months ahead + communicate well, it should be well received.

@luketaverne
Copy link
Collaborator

Added in f31e843 and several previous commits. Didn't think to squash them before the merge. If another contributor is better at git and wants to squash all of them into one commit, be my guest, I can't make it work around the merge :(

@nicolasembleton
Copy link
Collaborator

Actually @luketaverne I think you should do a pull request. I'm not sure why your commits don't show as such. Doing a pull request will automatically clean the commit history.

@luketaverne
Copy link
Collaborator

I think it's too late, I already pushed directly to this repo. I'll do a PR next time. Figured it didn't really make a difference with contributors. Sorry about that.

@nicolasembleton
Copy link
Collaborator

Haha. Yes. It's too late now. No worries @luketaverne . Continuous improvement :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants