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.

Sign up for GitHub

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

global: add support for index templates #148

Merged
merged 3 commits into from
Aug 14, 2024
Merged

global: add support for index templates #148

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4664a57
global: add support for index templates
zzacharo Jun 14, 2024
80b597a
ci: use reusable workflows
zzacharo Aug 14, 2024
0d7db4e
release: v4.1.0
zzacharo Aug 14, 2024
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
6 changes: 6 additions & 0 deletions invenio_stats/config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,3 +70,9 @@
delivery_mode="transient", # in-memory queue
)
"""Default exchange used for the message queues."""

STATS_REGISTER_INDEX_TEMPLATES = False
"""Register templates as index templates.

Default behaviour will register the templates as search templates.
"""
20 changes: 18 additions & 2 deletions invenio_stats/templates.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,13 @@

"""Celery background tasks."""

from flask import current_app

from .proxies import current_stats


def register_templates():
"""Register search templates for events."""
def _collect_templates():
"""Return event and aggregation templates from config."""
event_templates = [
event["templates"] for event in current_stats.events_config.values()
]
Expand All @@ -21,3 +23,17 @@ def register_templates():
]

return event_templates + aggregation_templates


def register_templates():
"""Register search templates for events."""
if current_app.config["STATS_REGISTER_INDEX_TEMPLATES"]:
return []
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure if it makes sense to show a deprecation warning without having a "way out". Usually these warnings link to some documentation that explains why the change is happening and how to migrate to the new solution (see example below).

I think to move things forward it would be better to cut a minor release, and include this deprecation warning in a new PR that would also include this extra information in the text (and release as a minor/patch).

GH Actions deprecation warning

image

SQLAlchemy

SADeprecationWarning: When transforming <class '__main__.User'> to a
dataclass, attribute(s) "create_user", "update_user" originates from
superclass <class
'__main__.Mixin'>, which is not a dataclass. This usage is deprecated and
will raise an error in SQLAlchemy 2.1. When declaring SQLAlchemy
Declarative Dataclasses, ensure that all mixin classes and other
superclasses which include attributes are also a subclass of
MappedAsDataclass.

return _collect_templates()


def register_index_templates():
"""Register search index templates for events."""
if not current_app.config["STATS_REGISTER_INDEX_TEMPLATES"]:
return []
Copy link
Contributor

Choose a reason for hiding this comment

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

these functions look a bit too similar and I am worried they will be easily confusing, but I don't have a good idea on how to avoid this. Maybe a bit more elaborate comments on both to explain the backwards compatibility of register_templates vs register_index_templates?
also I guess, we cannot raise errors instead of empty list, because we lose the backwards compatibility ?

return _collect_templates()
2 changes: 2 additions & 0 deletions setup.cfg
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,8 @@ invenio_base.api_blueprints =
invenio_stats = invenio_stats.views:blueprint
invenio_search.templates =
invenio_stats = invenio_stats.templates:register_templates
invenio_search.index_templates =
Copy link
Contributor

Choose a reason for hiding this comment

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

any way of installing one or the other, not both? maybe we need to deprecate invenio_search.templates?

invenio_stats = invenio_stats.templates:register_index_templates
invenio_queues.queues =
invenio_stats = invenio_stats.queues:declare_queues

Expand Down
Loading