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

cli: add index list option to init #192

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions invenio_search/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,16 +63,26 @@ def check():

@index.command()
@click.option('--force', is_flag=True, default=False)
@click.option(
'-i',
'--index_name',
ppanero marked this conversation as resolved.
Show resolved Hide resolved
'index_list',
help='List of indexes.',
multiple=True
)
@with_appcontext
@es_version_check
def init(force):
def init(force, index_list):
Copy link
Contributor

Choose a reason for hiding this comment

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

What do you think of adding the default value?

Suggested change
def init(force, index_list):
def init(force, index_list=[]):

or

Suggested change
def init(force, index_list):
def init(force, index_list=None):

"""Initialize registered aliases and mappings."""
ignore = [400] if force else None

click.secho('Creating indexes...', fg='green', bold=True, file=sys.stderr)
with click.progressbar(
current_search.create(ignore=[400] if force else None),
current_search.create(ignore=ignore, index_list=index_list),
length=len(current_search.mappings)) as bar:
for name, response in bar:
bar.label = name

click.secho('Putting templates...', fg='green', bold=True, file=sys.stderr)
with click.progressbar(
current_search.put_templates(ignore=[400] if force else None),
Expand Down
31 changes: 25 additions & 6 deletions invenio_search/ext.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
from .cli import index as index_cmd
from .errors import IndexAlreadyExistsError
from .utils import build_alias_name, build_index_from_parts, \
build_index_name, timestamp_suffix
build_index_name, timestamp_suffix, unsuffix_index


class _SearchState(object):
Expand Down Expand Up @@ -300,10 +300,24 @@ def create(self, ignore=None, ignore_existing=False, index_list=None):
elif ignore_existing and 400 not in ignore:
ignore.append(400)

def ensure_not_exists(name):
def ensure_index_not_exists(name):
if not ignore_existing and self.client.indices.exists(name):
raise IndexAlreadyExistsError(
'index/alias with name "{}" already exists'.format(name))
'index with name "{}" already exists'.format(name))

def ensure_alias_not_exists(name, indices=None):
if indices:
indices = ','.join(indices)

def _exists_alias(name, index):
return self.client.indices.exists_alias(name=name, index=index)

if not ignore_existing and _exists_alias(name=name, index=indices):
raise IndexAlreadyExistsError(
'alias with name "{}" already exists'.format(name))

def _unsuffix_indices_with_wildcard(indices):
return [unsuffix_index(index) + "*" for index in indices]

def _build(tree_or_filename, alias=None):
"""Build a list of index/alias actions to perform."""
Expand All @@ -319,10 +333,12 @@ def _build(tree_or_filename, alias=None):
ignore=ignore,
dry_run=True
)
ensure_not_exists(index_result[0])
ensure_index_not_exists(index_result[0])
new_indices[name] = index_result[0]
if alias_result[0]:
ensure_not_exists(alias_result[0])
ensure_alias_not_exists(alias_result[0])
ensure_index_not_exists(alias_result[0])

actions.append(dict(
type='create_index',
index=name,
Expand All @@ -341,7 +357,10 @@ def _build(tree_or_filename, alias=None):
]
if alias_indices:
alias_name = build_alias_name(alias, app=self.app)
ensure_not_exists(alias_name)
wildcard_indices = _unsuffix_indices_with_wildcard(
alias_indices
)
ensure_alias_not_exists(alias_name, wildcard_indices)
actions.append(dict(
type='create_alias',
index=alias_indices,
Expand Down
17 changes: 17 additions & 0 deletions invenio_search/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,23 @@ def suffix_index(index, suffix=None, app=None):
return index + suffix


def unsuffix_index(index, suffix=None, app=None):
"""Unsuffixes the given index.

:param index: Name of the index to prefix.
:param suffix: The suffix to remove from the index name.
:param app: Flask app to get the "invenio-search" extension from.
:returns: A string with the index name unsuffixed.
"""
search_ext = app.extensions['invenio-search'] if app else current_search
suffix = suffix if suffix is not None else search_ext.current_suffix
Copy link
Contributor

Choose a reason for hiding this comment

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

suffixes are also timestamps, and if they are not matched, then init creates a new index, I think we should also include this in the test cases (I saw it happening on integration testing)


if index.endswith(suffix):
index = index[:-len(suffix)]

return index


def build_index_from_parts(*parts):
"""Build an index name from parts.

Expand Down
32 changes: 32 additions & 0 deletions tests/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,38 @@ def test_init(app, template_entrypoints):

assert current_search_client.indices.exists(list(search.mappings.keys()))

# Test option --index_name
index = 'records-authorities-authority-v1.0.0'
suffixed_index = 'records-authorities-authority-v1.0.0-abc'
expected_aliases = [
'records',
'records-authorities',
'records-authorities-authority-v1.0.0'
]

assert current_search_client.indices.exists(suffixed_index)
result = runner.invoke(
cmd,
['delete', '--verbose', '--yes-i-know', suffixed_index],
obj=script_info
)
assert 0 == result.exit_code
assert not current_search_client.indices.exists(suffixed_index)
aliases = current_search_client.indices.get_alias()
assert suffixed_index not in list(aliases.keys())

result = runner.invoke(
cmd,
['init', '--index_name', index],
obj=script_info
)
assert 0 == result.exit_code
assert current_search_client.indices.exists(list(search.mappings.keys()))
aliases = current_search_client.indices.get_alias()
assert 8 == sum(len(idx.get('aliases', {})) for idx in aliases.values())
Copy link
Contributor

Choose a reason for hiding this comment

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

I had a lot of issues in the past (on cds-videos, another Invenio installation) with tests that contains these numbers, like 8 ==, because when changing something, it is hard to know what number to put there and why such number.

Could you try to replace the number with a named constant or maybe explain in a comment?

index_aliases = aliases[suffixed_index]['aliases'].keys()
assert expected_aliases == sorted(index_aliases)

# Clean-up:
result = runner.invoke(cmd, ['destroy'], obj=script_info)
assert 1 == result.exit_code
Expand Down