operation logs: use an Elasticsearch only resource #1937

jma · 2021-05-22T14:53:20Z

Renames virtua command CLI name.
Fixes monitor view to compute Elasticsearch and DB count diff when the
index does not exists.
Creates an operation logs Elasticsearch record class. It creates one
index per year.
Denies to all to read one record.
Adds a CLI to dumps the operation logs in a JSON file for backup.
Closes: Find a better operation log implementation #1725.

Co-Authored-By: Johnny Mariéthoz Johnny.Mariethoz@rero.ch

Why are you opening this PR?

Which task/US does it implement?
Which issue does it fix?

Dependencies

My PR depends on the following rero-ils-ui's PR(s):

rero/rero-ils-ui#

How to test?

What command should I have to run to test your PR?
What should I test through the UI?

Code review check list

iGormilhit

Commit message:

s/cli/CLI.
s/elasticsearch/Elasticsearch/.
s/db/DB/.

iGormilhit · 2021-05-25T05:17:49Z

rero_ils/modules/operation_logs/api.py


+def operation_log_id_fetcher(record_uuid, data):
+    """Fetch a Organisation record's identifiers.


Suggested change

"""Fetch a Organisation record's identifiers.

"""Fetch an Organisation record's identifier.

iGormilhit · 2021-05-25T05:18:47Z

rero_ils/modules/operation_logs/api.py

-        :param record_pid: record pid.
+    @classmethod
+    def create(cls, data, id_=None, index_refresh='false', **kwargs):
+        r"""Create a new record instance and store it in the database.


Is it database or ES index?

iGormilhit · 2021-05-25T05:19:49Z

rero_ils/modules/operation_logs/api.py

-        """Bulk index records.
+    @classmethod
+    def get_record(cls, id_):
+        """Retrieve the record by id.


Suggested change

"""Retrieve the record by id.

"""Retrieve the record by ID.

iGormilhit · 2021-05-25T05:20:38Z

rero_ils/modules/operation_logs/cli.py

+def dump_operation_logs(outfile):
+    """Dumps operation log records in a given file.
+
+    :param outfile: Json operation log output file.


Suggested change

:param outfile: Json operation log output file.

:param outfile: JSON operation log output file.

iGormilhit · 2021-05-25T05:21:09Z

rero_ils/modules/operation_logs/es_templates/__init__.py

@@ -1,7 +1,7 @@
 # -*- coding: utf-8 -*-
 #
 # RERO ILS
-# Copyright (C) 2021 RERO
+# Copyright (C) 2019 RERO


iGormilhit · 2021-05-25T05:21:54Z

rero_ils/modules/operation_logs/es_templates/__init__.py

@@ -15,6 +15,11 @@
 # You should have received a copy of the GNU Affero General Public License
 # along with this program. If not, see <http://www.gnu.org/licenses/>.

-"""Elasticsearch mappings."""
+"""Operation logs records elasticsarch templates."""


Suggested change

"""Operation logs records elasticsarch templates."""

"""Elasticsearch templates for Operation log records."""

iGormilhit · 2021-05-25T05:22:10Z

rero_ils/modules/operation_logs/es_templates/__init__.py

-from __future__ import absolute_import, print_function
+
+def list_es_templates():
+    """Elasticsearch Templates path."""


Suggested change

"""Elasticsearch Templates path."""

"""Elasticsearch templates path."""

iGormilhit · 2021-05-25T05:22:34Z

rero_ils/modules/operation_logs/es_templates/v7/__init__.py

@@ -1,7 +1,7 @@
 # -*- coding: utf-8 -*-
 #
 # RERO ILS
-# Copyright (C) 2021 RERO
+# Copyright (C) 2020 RERO


Why 2020 ? 🙂

iGormilhit · 2021-05-25T05:23:01Z

rero_ils/modules/operation_logs/extensions.py

+# You should have received a copy of the GNU Affero General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+"""Operation logs Record extensions."""


Suggested change

"""Operation logs Record extensions."""

"""Operation log record extensions."""

iGormilhit · 2021-05-25T05:24:28Z

tests/api/operation_logs/test_operation_logs_rest.py

+def test_operation_logs_rest(client, loan_pending_martigny,
+                             librarian_martigny,
+                             json_header):
+    """Test record retrieval."""


Same docstring as in L27 of the same file.

sebdeleze · 2021-05-26T06:06:19Z

rero_ils/modules/operation_logs/api.py

-        doc_types = None
-        fields = ('*', )
-        facets = {}
+LONG_INDEX_NAME = 'operation_logs-operation_log-v0.0.1'


Is it really used somewhere?

sebdeleze · 2021-05-26T06:19:14Z

tests/ui/operation_logs/test_operation_logs_api.py

+        OperationLog(operation_log_data).dumps()
+    tmp = deepcopy(operation_log_data)
+    tmp['date'] = '2020-01-21T09:51:52.879533+00:00'
+    OperationLog.create(tmp, index_refresh='wait_for')


I learned something (wait_for)

sebdeleze · 2021-05-26T06:32:05Z

rero_ils/modules/operation_logs/api.py

+                **kwargs
+            )
+
+        # TODO: enable this whe a invenio-records is updated.


Suggested change

# TODO: enable this whe a invenio-records is updated.

# TODO: enable this when a invenio-records is updated.

sebdeleze · 2021-05-26T06:38:03Z

rero_ils/modules/operation_logs/extensions.py

+        'holdings': 'hold'
+    }
+
+    def pre_dump(self, record, dumper=None):


Can this be done in the listener, or listener logic put here?

sebdeleze · 2021-05-26T06:39:31Z

rero_ils/config.py

@@ -1638,6 +1637,7 @@ def _(x):
        },
        record_class='rero_ils.modules.operation_logs.api:OperationLog',
        list_route='/operation_logs/',
+        # TODO: create a converter for es id, not used for the moment.
        item_route='/operation_logs/<pid(oplg, record_class='


I not used, it's better to remove.

sebdeleze · 2021-05-26T06:40:39Z

rero_ils/modules/fetchers.py

@@ -27,7 +27,7 @@


 def id_fetcher(record_uuid, data, provider, pid_key='pid'):
-    """Fetch a Organisation record's identifiers.
+    """Fetch an Organisation record's identifier.


Suggested change

"""Fetch an Organisation record's identifier.

"""Fetch a record's identifier.

sebdeleze · 2021-05-26T06:42:43Z

rero_ils/modules/operation_logs/api.py

-class OperationLogsIndexer(IlsRecordsIndexer):
-    """Operation log indexing class."""
+    @classmethod
+    def bulk(cls, data):


bulk_index is a better name

sebdeleze · 2021-05-26T06:45:16Z

rero_ils/modules/operation_logs/api.py

+        return cls(source.get('_source', {}))
+
+    @classmethod
+    def get_indices(cls):


Something like that can be used https://www.elastic.co/guide/en/elasticsearch/reference/7.12/cat-indices.html instead of a search ?

sebdeleze · 2021-05-26T06:46:51Z

rero_ils/modules/operation_logs/cli.py

+
+    :param outfile: JSON operation log output file.
+    """
+    enabled_logs = current_app.config.get('RERO_ILS_ENABLE_OPERATION_LOG')


Seems not be used.

sebdeleze · 2021-05-26T06:50:06Z

rero_ils/modules/operation_logs/cli.py

+@click.command('dump_operation_logs')
+@click.argument('outfile', type=click.File('w'))
+@with_appcontext
+def dump_operation_logs(outfile):


Should be useful to produce only a dump for a specific year?

lauren-d

no more comment

* Renames virtua command CLI name. * Fixes monitor view to compute Elasticsearch and DB count diff when the index does not exists. * Creates an operation logs Elasticsearch record class. It creates one index per year. * Denies to all to read one record. * Adds a CLI to dumps the operation logs in a JSON file for backup. * Closes: rero#1725. Co-Authored-By: Johnny Mariéthoz <Johnny.Mariethoz@rero.ch>

jma force-pushed the maj-record-es branch 4 times, most recently from b381c66 to 9ab6b1e Compare May 23, 2021 08:02

jma changed the title ~~in progress~~ operation logs: use an elasticsearch only resource May 23, 2021

jma force-pushed the maj-record-es branch 2 times, most recently from d7e0bd5 to 1b60fce Compare May 24, 2021 08:04

jma requested review from lauren-d and sebdeleze May 24, 2021 08:06

jma marked this pull request as ready for review May 24, 2021 08:06

iGormilhit reviewed May 25, 2021

View reviewed changes

iGormilhit added the f: activity-logs Everything around logging user or system activities label May 25, 2021

iGormilhit assigned jma May 25, 2021

jma force-pushed the maj-record-es branch 2 times, most recently from f10c903 to e804e37 Compare May 26, 2021 06:49

sebdeleze approved these changes May 26, 2021

View reviewed changes

lauren-d approved these changes May 26, 2021

View reviewed changes

jma force-pushed the maj-record-es branch 2 times, most recently from 4ffc736 to b869885 Compare May 26, 2021 09:07

jma requested a review from iGormilhit May 26, 2021 09:47

jma force-pushed the maj-record-es branch 3 times, most recently from 8c88771 to 5661fdc Compare May 26, 2021 13:24

jma changed the title ~~operation logs: use an elasticsearch only resource~~ operation logs: use an Elasticsearch only resource May 26, 2021

jma force-pushed the maj-record-es branch from 5661fdc to afc478d Compare May 27, 2021 05:15

iGormilhit approved these changes May 27, 2021

View reviewed changes

jma merged commit df3a6c0 into rero:maj-US1913-log-loan-history May 27, 2021

jma deleted the maj-record-es branch January 13, 2022 14:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

operation logs: use an Elasticsearch only resource #1937

operation logs: use an Elasticsearch only resource #1937

jma commented May 22, 2021 •

edited

Loading

iGormilhit left a comment

iGormilhit May 25, 2021

iGormilhit May 25, 2021

iGormilhit May 25, 2021

iGormilhit May 25, 2021

iGormilhit May 25, 2021

iGormilhit May 25, 2021

iGormilhit May 25, 2021

iGormilhit May 25, 2021

iGormilhit May 25, 2021

iGormilhit May 25, 2021

sebdeleze May 26, 2021

sebdeleze May 26, 2021

sebdeleze May 26, 2021

sebdeleze May 26, 2021

sebdeleze May 26, 2021

sebdeleze May 26, 2021

sebdeleze May 26, 2021

sebdeleze May 26, 2021

sebdeleze May 26, 2021

sebdeleze May 26, 2021

lauren-d left a comment


		def operation_log_id_fetcher(record_uuid, data):
		"""Fetch a Organisation record's identifiers.

	"""Fetch a Organisation record's identifiers.
	"""Fetch an Organisation record's identifier.

	:param outfile: Json operation log output file.
	:param outfile: JSON operation log output file.

	"""Operation logs records elasticsarch templates."""
	"""Elasticsearch templates for Operation log records."""

	"""Elasticsearch Templates path."""
	"""Elasticsearch templates path."""

	"""Operation logs Record extensions."""
	"""Operation log record extensions."""

	# TODO: enable this whe a invenio-records is updated.
	# TODO: enable this when a invenio-records is updated.

	"""Fetch an Organisation record's identifier.
	"""Fetch a record's identifier.

operation logs: use an Elasticsearch only resource #1937

operation logs: use an Elasticsearch only resource #1937

Conversation

jma commented May 22, 2021 • edited Loading

Why are you opening this PR?

Dependencies

How to test?

Code review check list

iGormilhit left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lauren-d left a comment

Choose a reason for hiding this comment

jma commented May 22, 2021 •

edited

Loading