-
Notifications
You must be signed in to change notification settings - Fork 11
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
prefer bibgroup and bibgroup_facets from nonbib #159
Conversation
and update pipeline utils release to get latest protobufs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes are simple enough, but before merging, please, check the comments, especially the one about python 3 and the requirements.
@@ -1,4 +1,4 @@ | |||
adsputils==1.2.8 | |||
adsputils==1.3.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this version is python 3 compatible only, which would make this version of master python 3 compatible only. You may want to hold this change for when you decide to deploy master with Python 3.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll add python3 support to this pull request.
adsmp/solr_updater.py
Outdated
if db_record.get('nonbib_data', None) and db_record['nonbib_data'].get('bibgroup'): | ||
out['bibgroup'] = db_record['nonbib_data']['bibgroup'] | ||
if db_record.get('nonbib_data', None) and db_record['nonbib_data'].get('bibgroup_facet'): | ||
out['bibgroup_facet'] = db_record['nonbib_data']['bibgroup_facet'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are repeating the same condition verification twice (i.e., db_record.get('nonbib_data', None)
), we could have a nested if to remove that dup. Also, given that we are not using the returned value, we could just use if 'nonbib_data' in db_record
(as you prefer).
also requires changes to file /app/reindex.py which is in BeeHive
and update pipeline utils release to get latest protobufs