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

Added options to alias creation #161

Closed
wants to merge 11 commits into from
Closed

Added options to alias creation #161

wants to merge 11 commits into from

Conversation

comulinux
Copy link
Contributor

Hi Ruflin,
I'm trying to implement the alias API as is described in

http://www.elasticsearch.org/guide/reference/api/admin-indices-aliases.html.

This is my first approach. With this we have functionallity to make filtered aliases and add routing in alias creation (for searching and indexing)
I can't find a solution for the Elastica_Exception_NotImplemented part, but we can make a filtered alias passing the filter as a string


case 'filter':
if (is_a($value, 'Elastica_Query'))
throw new Elastica_Exception_NotImplemented('Apply a query object to an alias is not implemented yet.');
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Elasticsearch documentation says: "The filter can be defined using Query DSL" so I think that this function must be prepared to take a Elastica_Query as a filter, but I don't know how to obtain a text with the query :-/

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you need a text here? Or what do you mean exactly? I would assume that you can use Elastica_Query::toArray() and add it to the array.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May be I'm building the query incorrectly (I have tried in several ways) because when I made this the result was:

curl -XPOST http://localhost:9200/_aliases -d '{"actions":[{"add":{"index":"test","alias":"test-aliase","index_routing":"1","search_routing":"1,2","filter":{}}}]}'

If you see the test, my idea was to build the same query that use the test with a string but with a Elastica_Query object.
Now I'm trying with:
$query = new Elastica_Query_Term();
$query->setTerm('user', 'comulinux');

And I make:

$add['filter'] = $value->toArray();

With the result commented, any idea?

@comulinux
Copy link
Contributor Author

By the way, I don't know what the hell is SearchTest doing here as a modified file...

$status = new Elastica_Status($client);
$this->assertTrue($status->aliasExists($aliasName));
$this->fail('Should throw Elastica_Exception_NotImplemented');
}catch(Exception $ex){
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here you could directly catch the exception Elastica_Exception_NotImplemented, so all the other ones would show up in phpunit as a fail.

}else{
$add['filter'] = $value->toArray();
}
break;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's the easy way. I prefer a "is_a($value, 'Elastica_Query_XXX)" but I don't know how to ask about all the Elastica_Query classes.
By the way, it seems to work :)

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if ($value instanceof Elastica_Query_Abstract)

This should work ;-)

@ruflin
Copy link
Owner

ruflin commented Jul 14, 2012

@comulinux @kmc Sorry, somehow I lost track here. How are your pull request / changes going to conflict? Can of you two guys merge these to pull requests to make sure they work together? Probably it is simpler to merge #172 into #161.

@ruflin
Copy link
Owner

ruflin commented Dec 23, 2012

@comulinux At the moment I'm refactoring Elastica. I also took again a closer look at this pull request. I have the following suggestion. I would add an object Elastica_Alias which handles all the complexity. and is also responsible to create and remove the alias. addAlias in Elastica_Index would only be a proxy. Like this it would also be possible to add multiple Indices to an Alias etc. in one call.

@ruflin
Copy link
Owner

ruflin commented Jan 13, 2013

I will close this pull request as these changes partially did already go into the code and will be completely resolved by issue #297

@ruflin ruflin closed this Jan 13, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants