django-geo-spaas
django-geo-spaas copied to clipboard
get_or_create in nansat_ingestor should not use update_or_create
In geospaas.nansat_ingestor.managers.get_or_create
, you use Django's update_or_create
method. This is confusing and should be changed. Perhaps adding a second manager method update_or_create
?
I think get_or_create
is still needed, since a user doesn't necessarily want to update an existing entry.
Thanks for comment, @mortenwh ! I agree that using update_or_create
inside get_or_create
is confusing. Maybe we should rather change the name geospaas.nansat_ingestor.managers.get_or_create
to ingest
or something similar to avoid confusion. Because in fact this is a very large method that extracts metadata from nansat object and saves into database. Ideally it should be split into more smaller methods.
But we have to use update_or_create
inside - now there can be several sources of information for one record: one from django-geo-spaas-harvesting, another one (and more complete) from nansat ingestor.
I see - so does it mean that it would be better to use the harvesting package for initial ingestion (e.g., before processing)?
On Tue, Aug 4, 2020 at 10:37 AM Anton Korosov [email protected] wrote:
Thanks for comment, @mortenwh https://github.com/mortenwh ! I agree that using update_or_create inside get_or_create is confusing. Maybe we should rather change the name geospaas.nansat_ingestor.managers.get_or_create to ingest or something similar to avoid confusion. Because in fact this is a very large method that extracts metadata from nansat object and saves into database. Ideally it should be split into more smaller methods.
But we have to use update_or_create inside - now there can be several sources of information for one record: one from django-geo-spaas-harvesting, another one (and more complete) from nansat ingestor.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/nansencenter/django-geo-spaas/issues/127#issuecomment-668464152, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAA2UBICT4PFZPYYLGHGU3TR67CFBANCNFSM4PGQQ5EA .
-- Morten Wergeland Hansen, PhD Meteorologisk Institutt / Norwegian Meteorological Institute T.: (+47) 915 47 844
Yes, you got it right. I'd rather keep this issue open to remind to refactor get_or_create some time later.