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

Already on GitHub? Sign in to your account

Renaming the Perl module #20

Closed
clintongormley opened this Issue Mar 3, 2014 · 41 comments

Comments

Projects
None yet
Owner

clintongormley commented Mar 3, 2014

In September 2013 I released the official Perl client: Elasticsearch.pm and deprecated my old client which was called ElasticSearch.pm (note the capital S).

At the time I was under the impression that PAUSE was case-sensitive and thought that the similarity of the names might cause a small human problem but wouldn't be a technical problem. Shortly after release, @karenetheridge pointed out to me (elasticsearch#5) that users of case insensitive filesystems (Windows and OSX) would be unable to install both versions of the module at the same time, making migration problematic.

I then released Elasticsearch::Compat, which provides the old API but uses the new Elasticsearch.pm client internally, hoping that this would ease migration. However, there are a number of modules that depend on the old client and so conflicts continue to occur. This issue was raised again last week in elasticsearch#18.

All of the official clients are named Elasticsearch so I was loathe to change the name, but after talking the problem through with the cool headed @neilbowers (++), he convinced me of the need to:

  1. Deprecate both ElasticSearch.pm and Elasticsearch.pm and,
  2. To release the official client under a new name with no possibility of clashes.

This will cause some pain (for which I apologise) to users who have already migrated to the new Elasticsearch.pm as they will need to update their code to use the new name, but it is the better long term solution.

What should it be called?

I would like feedback from users of the module as to what the new name should be. Suggestions so far include Search::Elasticsearch and Elasticsearch::V2. Any other suggestions are welcome. Please vote for your preferred name in the comments below.

See the blogpost for more details about the future plans for elasticsearch-perl.

disem commented Mar 3, 2014

How about Perlastic ? :)

labrown commented Mar 3, 2014

Elastiperl ? ;-)

Owner

clintongormley commented Mar 3, 2014

@disem @labrown I'd prefer to keep the full word Elasticsearch in the name as it makes it easier to search for.

jprante commented Mar 3, 2014

use Bonsai::Elasticsearch;

awwaiid commented Mar 3, 2014

Search::Elasticsearch seems straightforward

labrown commented Mar 3, 2014

The Search:: namespace is quite active and Search::Elasticsearch is a good descriptive name.

There's a perk of the Search:: namespace, and that its primarily that you'll not need to worry about this hazzard:

- user has previously installed ElasticSearch::Foo  in $PREFIX/ElasticSearch/Foo.pm
- Elasticsearch::V2 comes out
- needs $PREFIX/Elasticsearch/v2.pm
- .... sharing the path part with the sensitivity difference

I'm not sure if that is, or isn't a problem, but I'd hope to avoid it if it is.

Contributor

oalders commented Mar 3, 2014

Search::Elasticsearch works for me.

If this module uses the REST API of ElasticSearch then wouldn't the correct place be WWW::Elasticsearch, as that namespace is supposed to deal with clients towards web services...? But I'm all for sticking it in Search:: as well.

smonff commented Mar 3, 2014

Search::Elastic and Elastic::Search save a lot of typing.

Contributor

ranguard commented Mar 3, 2014

Search::Elasticsearch or WWW::Elasticsearch

timbunce commented Mar 3, 2014

Search::Elasticsearch.

Member

kurtado commented Mar 3, 2014

+1 on Search::Elasticsearch

bfaria commented Mar 3, 2014

What about plastic

ctreptow commented Mar 3, 2014

Ditto for Search::Elasticsearch.

abraxxa commented Mar 4, 2014

Seems Search::Elasticsearch is preferred by most, works for me too.

monken commented Mar 4, 2014

How about Elasticsearch::Client. You are essentially using that namespace already, just move the content of Elasticsearch.pm to Elasticsearch/Client.pm.

+1 Elasticsearch::Client

Search::Elastic (note the absence of a tautology) seems practical but boring. I rather like bonsai but it is a bit obscure. My suggestion is perles.

Elasticsearch::Client still risks the path-part ambiguity on case insensitive OSes.

Because ElasticSearch/ will exist first, and so Elasticsearch::Client will be installed as ElasticSearch/Client.pm # with the upper case S in the path part

Thats not as big a problem as it was, but it still is concern scope for me.

haarg commented Mar 4, 2014

The directory names matching case insensitively isn't a significant problem, since a require Elasticsearch::Client will still find ElasticSearch/Client.pm. I wouldn't let that impact a decision on the name as long at the the full names aren't ambiguous.

Owner

clintongormley commented Mar 4, 2014

@monken It's not just moving Elasticsearch to Elasticsearch::Client. There are also clashes on eg Elastic[Ss]earch::Transport and maybe a couple of other places. And while we don't have Elasticsearch::Client per se, we do have Elasticsearch::Client::Direct.

@michaelsalmon et al: The name of the server is Elasticsearch, so I'd prefer to use the full name rather than just Elastic. It'd also be better when searching for the module.

The majority seems to be in favour of Search::Elasticsearch. It is clear what it does, and has the benefit of a complete change in namespace, avoiding any potential collisions. While it is a little longer than before, in reality you only need to type the full name once or twice in any application, as the rest of the time you'll just need to reference the $es object.

Thanks to all who have participated. Let the renaming begin!

diegok commented Mar 5, 2014

Oh!, i'm late... I was going to give a +1 to Elasticsearch::Client and to propose that Elasticsearch::Compat becomes ElasticSearch :-)

@vpeil vpeil referenced this issue in LibreCat/Catmandu-Store-Elasticsearch Mar 6, 2014

Closed

Switch to Search::Elasticsearch #2

Owner

clintongormley commented Mar 6, 2014

Just a note reporting progress thus far:

I've released:

I've uploaded these dists with deprecation notices:

Still to be deprecated:

Still to be migrated:

abraxxa commented Mar 6, 2014

Thanks for that massive amount of work Clinton! I'm looking forward to releasing Message::Passing::Output::Elasticsearch which is already on my github account but is waiting on the async feature of the new library.
Name suggestions to avoid the same problem welcome! (The current one is named Message::Passing::Output::ElasticSearch).

@felliott felliott added a commit to Judoon/judoon that referenced this issue Mar 10, 2014

@felliott felliott [cpanfile] depend on Elastic::Model => 0.28
 * Elastic::Model v0.28 fixes several important bugs:

   * It stops using the deprecated Class::MOP::class_load, removing the
     need to have a locally patched Elastic::Model::TypeMap::Base

   * It depends on the new Search::Elasticsearch family of modules, a
     renaming of the Elastic[Ss]earch modules that were causing issues
     w/ case-insensitive filesystems.  See:
     elastic/elasticsearch-perl#20

 * As a side-effect, this should make Judoon carton-able now (I think, I hope)
c78cdef

fciuciu commented Apr 7, 2014

Does your module support regular expressions?

Owner

clintongormley commented Apr 23, 2014

@abraxxa Sorry for how long it has taken, but I've just released Search::Elasticsearch::Async - an async version of the ES client which uses Promises.

See https://metacpan.org/pod/release/DRTECH/Search-Elasticsearch-Async-1.11/lib/Search/Elasticsearch/Async.pm

I hope the docs are clear enough, but ping me if you need any help. Promises can take a while to get used to, but this helps a lot: https://metacpan.org/pod/distribution/Promises/lib/Promises/Cookbook/GentleIntro.pod

abraxxa commented Apr 23, 2014

Great, thank you Clinton!
I'm already using the sync version in production quite sucessfully, although about 35% slower than the old module.
Will look into the async one in the next week and change Message::Passing::Output::Search::Elasticsearch to use it.

abraxxa commented Apr 24, 2014

@clintongormley just found Search::Elasticsearch::Async::Bulk, note that the pod links to Elasticsearch::Async::Bulk which doesn't exist.

abraxxa commented Apr 24, 2014

@clintongormley it would help if the instance returned by Search::Elasticsearch::Async had the Search::Elasticsearch::Role::Is_Async role applied just like Search::Elasticsearch::Async::Bulk.

Owner

clintongormley commented Apr 24, 2014

@abraxxa Thanks - fixed the bad POD links.

Re applying the role to the Client classes, not sure about that. I could apply them to the client object itself... but you can always check

$es->transport->does('Search::Elasticsearch::Role::Is_Async')
Owner

clintongormley commented Apr 24, 2014

@abraxxa Re your comment about the slower performance of the new module, I'd love to see your test for that, so that I can work on improving performance.

Could you open a separate issue with the details?

abraxxa commented Apr 24, 2014

If you look at the current version of the module it allows to pass an ES and as well a ::Bulk object or let the Elasticsearch output instantiate them.
I'd like to make sure that a passed object is a Search::Elasticsearch::Async one which I can't check because the returned instance is a ::Client::Direct object.

Regarding performance: with the old Message::Passing::Output::ElasticSearch I pushed up to 1.500 messages/second from RabbitMQ to ElasticSearch on the same box with some metadata enrichment inbetween. With the new Message::Passing::Output::Search::Elasticsearch I only got about 1.000 messages/second.
Let's see how this value changes when using the new async code.

abraxxa commented Apr 25, 2014

@clintongormley converting https://github.com/abraxxa/Message-Passing-Output-Search-Elasticsearch to Async was really easy, the hardest part was the test case and realizing that I had to start the AnyEvent loop to make it do anything. Please review my code and test, if you give the go I'd release it to CPAN.
If already put it into production, works great and the performance is up to 1.500 messages/second. @bobtfish said that the AMQP decoding will be the bottleneck in my setup.

Owner

clintongormley commented Mar 10, 2015

Nooooooooooooooo
On 10 Mar 2015 13:51, "Leo Lapworth" notifications@github.com wrote:

Time to rename again... Elastic::Stuff :) ?

https://www.elastic.co/about/press/elasticsearch-changes-name-to-elastic-to-reflect-wide-adoption-beyond-search


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

Contributor

ranguard commented Mar 11, 2015

You could probably close this issue then ;)

Actually, it's just the company that has changed name to "Elastic", the database/search backend is still called Elasticsearch (from reading the press release and looking at their product list on elastic.co.

Contributor

ranguard commented Mar 11, 2015

@robinsmidsrod yea, I knew that.. but wasn't so funny :)

garu commented May 21, 2015

Hi @clintongormley! As @ranguard pointed out, shouldn't this issue be closed?

Owner

clintongormley commented May 22, 2015

Indeed - I've deleted the various old variations of module names, leaving only Search::Elasticsearch::*

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment