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

SEARCH-681: Index genres #137

Draft
wants to merge 1 commit into
base: released-master
Choose a base branch
from

Conversation

reosarevok
Copy link
Member

Implement SEARCH-681

AFAICT this is all that needs to be changed, except for generating new triggers and whatnot so that they check genre_annotation. But I didn't manage to generate them locally with the command in a8798bc - dunno if that's changed or something is missing here (it complains about ModuleNotFoundError: No module named 'sentry_sdk' but I also can't run setup properly, it seems, because then it complains about No such file or directory: 'mbrng/init.py). Maybe someone else can generate them and squash?

Copy link
Contributor

@yvanzo yvanzo left a comment

Choose a reason for hiding this comment

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

The code changes look good so far but I didn’t test it either.

The reindex couldn’t work because of a bug in the commit dff0a11 it was based on. A fix for this bug is in master but it depends on code migrated to SQLAlchemy 1.4 and which isn’t production-ready yet. So as a workaround, this pull request can be based on the temporary branch released-master (which match the latest release v3.0.1) until master gets stabilized.

It requires that the genre core exists. This core will be defined by running a new release of mb-solr that makes use of metabrainz/mbsssss#61. SIR reindex will fail without that.

Note that the latest release v-2020-11-12 of mmmd-schema doesn’t have genre’s annotation, aliases, and relationships. So the upcoming release of mb-solr should also use a new release of mmd-schema to fully implement SEARCH-680 and SEARCH-681 at once.

@reosarevok reosarevok changed the base branch from master to released-master December 6, 2022 14:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants