Page MenuHomePhabricator

Make it possible to configure Elasticsearch index name
ClosedPublic

Authored by WikiChad on Jul 2 2014, 7:19 PM.
Tags
None
Referenced Files
F14357482: D9798.diff
Fri, Dec 20, 5:22 AM
Unknown Object (File)
Wed, Dec 18, 8:57 AM
Unknown Object (File)
Wed, Dec 18, 3:25 AM
Unknown Object (File)
Sun, Dec 15, 11:16 PM
Unknown Object (File)
Thu, Dec 12, 6:43 PM
Unknown Object (File)
Sat, Dec 7, 3:49 PM
Unknown Object (File)
Sat, Nov 30, 8:30 PM
Unknown Object (File)
Tue, Nov 26, 4:51 PM

Details

Summary

Similar to storage.default-namespace sometimes during development you'll want
to handle multiple indexes alongside one another. Rather than hardcoding the
/phabricator/ index make this exposed in new search.elastic.index setting,
defaulting to the existing "phabricator"

Test Plan

Existing installations should be unaffected by this change. Changing the new
setting will result in new indexes being created when someone runs
./bin/search index again

Diff Detail

Repository
rP Phabricator
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

WikiChad retitled this revision from to Make it possible to configure Elasticsearch index name.
WikiChad updated this object.
WikiChad edited the test plan for this revision. (Show Details)
WikiChad added a reviewer: epriestley.
epriestley added a subscriber: 20after4.

+ @20after4

WMF should be able to sign the corporate CLA now (L30) -- I can add exemptions as necessary after that to clear the signature block.

I went ahead and signed the individual CLA since I retain the copyright on all my work.

epriestley edited edge metadata.

Two really minor inlines. I think search.elastic.namespace might also be clearer than serach.elastic.index. That said, I'm not very familiar with ElasticSearch. Is "index" likely to be clearer to users who are?

conf/default.conf.php
764–766 ↗(On Diff #23517)

This file is on its way out -- there's a brief message in the header, but basically just don't add anything here. We can wipe the file at some point but need to make sure that all the settings in this file are exactly the same as the default settings in the code so that wiping it won't change installs.

src/applications/search/engine/PhabricatorSearchEngineElastic.php
242–245

This should probably be:

$uri = new PhutilURI($this->uri);
$uri->setPath($this->index);
$uri->appendPath($path);

That should get all the various missing-trailing-slash cases correct, I think.

This revision now requires changes to proceed.Jul 9 2014, 10:40 PM
WikiChad edited edge metadata.

Handle uri formation better, don't add to deprecated config, call the
setting 'namespace' so it's more clear to Phabricator.

This revision is now accepted and ready to land.Jul 11 2014, 1:40 AM
epriestley updated this revision to Diff 23739.

Closed by commit rP66a3abe058a7 (authored by @WikiChad, committed by @epriestley).