Skip to content

Data Package should contain a publishers.jl file too #269

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

Open
wants to merge 1 commit into
base: live
Choose a base branch
from

Conversation

jarofgreen
Copy link
Contributor

@jarofgreen jarofgreen self-assigned this Mar 26, 2025
@jarofgreen
Copy link
Contributor Author

The aggregates on publisher are updated by the "rewrite_quality_data" command, not the "manage_entities_data" command - maybe this should be added to the "Updating entities data" section in the Readme?

Also, I can see Publisher's are created by https://github.com/ThreeSixtyGiving/datastore/blob/live/datastore/db/management/commands/load_datagetter_data.py#L142 but I can't see where they are deleted.
In some places Publishers are got by data run https://github.com/ThreeSixtyGiving/datastore/blob/live/datastore/data_quality/management/commands/rewrite_quality_data.py#L113 , I assume this is to make sure old publishers aren't processed. Should we be doing something here to make sure old publishers aren't included?

@jarofgreen jarofgreen requested a review from michaelwood March 26, 2025 11:07
@jarofgreen
Copy link
Contributor Author

In some places Publishers are got by data run , I assume this is to make sure old publishers aren't processed.

Also, on reading the code I wasn't certain getter_run was being updated properly on publisher.

https://github.com/ThreeSixtyGiving/datastore/blob/live/datastore/db/management/commands/load_datagetter_data.py#L142 uses get_or_create to populate the publisher variable, but I couldn't see anywhere in the following code that the publisher variable is actually saved.

There is an update_or_create method that may be what is wanted here?

But this is just from reading code, I didn't test to double check this.

@@ -87,20 +87,32 @@ def create_orgs_list(entity_type, output=sys.stdout):
entity_type: publisher, recipient, funder
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Interesting comment, I wasn't sure if it ever worked in the past or was just planned to work at some point in the future

LEFT OUTER JOIN additional_data_orginfocache on db_{entity_type}.org_id = additional_data_orginfocache.org_id
LEFT OUTER JOIN db_publisher on db_{entity_type}.org_id = db_publisher.org_id OR db_publisher.org_id = ANY(db_{entity_type}.non_primary_org_ids)
"""
if entity_type == "publisher":
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did try one query var and several ("" if x else "") in the middle but it seemed like this way was way more readable.

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.

1 participant