Very unfortunate choice of module name

[karenetheridge]

Elasticsearch.pm seems to be a redo of ElasticSearch.pm, with a new interface. Unfortunately, because these names are identical except for case, it is impossible for both modules to be installed at the same time on case-insensitive file systems -- and worse, the wrong module can be loaded, which can lead to runtime errors due to calling the wrong interface.

Since Elasticsearch is very new, is it possible to move it to a different namespace that will not collide with ElasticSearch?

[clintongormley]

Hi Karen

Both modules were written by me, and are pretty similar in nature, although the newer one is more maintainable and extensible.

The name clash is an unfortunate result of not having had sufficient time to add all the extras that the old module has before needing to release the new module.

I'm currently working on more backend transports: LWP, Net::Curl, and an async version which will support AnyEvent and Mojo via Promises.

Then I plan on releasing a ::Company module to mitigate the transition for users of the old module.

I plan on renaming the old module to something like Legacy::ElasticSearch to solve the issue of namespace clashes on case insensitive file systems, but I was going to wait until ES::Compact was ready. But perhaps I should go ahead and rename it now.

Any thoughts on this, or alternative naming?

Clint

[clintongormley]

Bah... Autocorrect. That's Compat, not Compact nor Company :)

[karenetheridge]

I plan on renaming the old module to something like Legacy::ElasticSearch

If you re-release ElasticSearch with a deprecation warning, advising a migration to Legacy::ElasticSearch, then you can get people off the old name fairly quickly (as soon as they upgrade from cpan at least), but people who install just Elasticsearch (or code that uses it) won't see that deprecation warning, and will still have their ElasticSearch.pm clobbered by the new install.

The only safe thing to do is to rename the new Elasticsearch to something that won't conflict with existing installations.

[karenetheridge]

I posted this issue to the module-authors list in case there is another solution that I've missed.

[karenetheridge]

people who install just Elasticsearch (or code that uses it)

You could potentially do some checking at install time (i.e. via Makefile.PL) to see if there is an existing ElasticSearch.pm and refuse to install until it is removed and Legacy::ElasticSearch is installed in its place...

[clintongormley]

The CPAN toolchain IS case sensitive, so people won't install the wrong module by mistake. Even if they manage to do so, as soon as they try to use ElasticSearch they will get an error, so I see the major issue being one of annoyance. Unfortunate, I agree, but not irreparable.

[karenetheridge]

The CPAN toolchain IS case sensitive, so people won't install the wrong module by mistake

You're assuming they're installing the module directly, rather than indirectly as a prereq for something else (or via some other tool that installs lots of modules, e.g. a carton file, or a darkpan installation). The user won't necessarily be aware ahead of time that their existing ElasticSearch is going to get overwritten.

This isn't hypothetical - this already happened at my company last week.

[karenetheridge]

Also, ElasticSearch.pm is no longer indexed, because PAUSE indexes case-insensitively (if another author had uploaded Elasticsearch.pm, they would have gotten an indexing error - i.e. an unauthorized dist -- but since you own both dists, Elasticsearch.pm replaced ElasticSearch.pm in the index.)

[clintongormley]

Re install time checking, I thought about that but thought I'd be more likely to mess that up and cause more problems. But perhaps I should change ElasticSearch to error on install and give instructions to use Legacy:: instead.

Re CPAN indexing: both versions of the module show up as indexed. See http://search.cpan.org/search?query=elasticsearch&mode=all and https://metacpan.org/search?q=elasticsearch.

And installing modules via (at least) cpanminus is case sensitive as well.

So unless there is something that I've missed, things seem to work as expected, no?

[clintongormley]

What happened at your company? A problem with installing ES? If so, then apparently I have missed something. Any details of the tool chain etc?

[clintongormley]

The other thing I could do is to die with an explanatory message if somebody tried to instantiate ElasticSearch after using Elasticsearch.

[karenetheridge]

perhaps I should change ElasticSearch to error on install and give instructions to use Legacy:: instead.

Again, that won't help anyone who already has ElasticSearch installed and they install Elasticsearch instead.

Re CPAN indexing: both versions of the module show up as indexed. See http://search.cpan.org/search?query=elasticsearch&mode=all and https://metacpan.org/search?q=elasticsearch.

Neither of those are querying the canonical index. Please see: http://cpan.perl.org/modules/02packages.details.txt which is what the cpan clients use to map module names to dist tarball names.

The other thing I could do is to die with an explanatory message if somebody tried to instantiate ElasticSearch after using Elasticsearch

Perhaps - you'd have to look in %INC to see what file actually got loaded, and attempt to infer what module they actually wanted based on that. You'd also need to put the check in both ElasticSearch.pm and Elasticsearch.pm. This won't help anyone with existing installations.

[karenetheridge]

What happened at your company? A problem with installing ES?

Someone noticed that the new API had been released, wrote some new code against that API, and added 'Elasticsearch' to the script that installs all dependencies. Other code was still using ElasticSearch's APIs. kaboom.

[clintongormley]

Hmmm.. you appear to be right! My apologies. And I hadn't considered the overwriting aspect of it. OK, I will do something about this asap.

[abraxxa]

Subscribing to the issue as I'm in the process of writing Message::Passing::Output::Elasticsearch.

[clintongormley]

OK, this is what I've done to sort out this problem:

1. Elasticsearch::Compat https://metacpan.org/module/Elasticsearch::Compat

This module uses the new Elasticsearch client internally, but the user facing API is taken entirely from the old module. So you should be able to use your old code alongside the new code even on a case-insensitive file system as follows:

use Elasticsearch;
use Elasticsearch::Compat;

my $new_es = Elasticsearch->new(...); 
my $old_es = Elasticsearch::Compat->new(...);

2. More backends

I have added an LWP backed https://metacpan.org/module/Elasticsearch::Cxn::LWP and a Net::Curl backend https://metacpan.org/module/Elasticsearch::Cxn::NetCurl to the existing HTTP::Tiny backend.

Currently there is no support for AnyEvent backends. I'm working on a new Elasticsearch::Async, which will use Promises to wrap AnyEvent or Mojo, but will probably not be able to include these cleanly in the Compat module.

3. Detect loading of Elasticsearch when the user meant ElasticSearch

On case insensitive file systems, Elasticsearch can overwrite ElasticSearch, and a use ElasticSearch; can load the new version instead. In these situations, we now die when the user calls ElasticSearch->new: see b855ae9

However, on case-sensitive file systems, the user will be able to load both modules at the same time and use them independently.

[kentfredric]

I'm not understanding how that conclusion sorts out anything.

Its basically saying "If you want to keep using ElasticSearch.pm, you'll have to rewrite all your code to say Elasticsearch::Compat".

That is very much "not ok", and all the attempts at avoiding breaking API are completely obliterated by that.

If you have to rewrite half your code to avoid API breakage, API has been broken.

Same with your "die if its ElasticSearch.pm" logic. You're explicitly breaking existing code with that.

IMO, The Best solution is to stop the sillyness of trying to keep it as "Elasticsearch" and rename it something sensible, like "ElasticSearch2" or even "Elasticsearch2", really, anything that doesn't invoke a case-sensitivity collision.

And the longer you leave fixing this, the worse the problem will get, because you'll have a growing stack of things depending on the new name, and they are going to experience collisions due to the existence of the old name.

And its not just filesystem insensitivity that is an issue, PAUSE indexing is an issue.

As is Human recognition, because its hard for a human to visually realise "Well, this one has a lower case s, the other has an upper case S", and hard to realise the difference that implies.

You may consider the pain of having to move away from this "new" name already significant, but I assure you, that pain will be tiny compared to the aggregate pain of people hitting this problem on a regular basis, and filing bogus bug reports about this problem, and the problem getting worse and harder to move away from.