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

get_or_create in nansat_ingestor should not use update_or_create #127

Open
mortenwh opened this issue Jul 24, 2020 · 3 comments
Open

get_or_create in nansat_ingestor should not use update_or_create #127

mortenwh opened this issue Jul 24, 2020 · 3 comments

Comments

@mortenwh
Copy link
Contributor

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.

@akorosov
Copy link
Member

akorosov commented Aug 4, 2020

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.

@mortenwh
Copy link
Contributor Author

mortenwh commented Aug 4, 2020 via email

@akorosov
Copy link
Member

Yes, you got it right.
I'd rather keep this issue open to remind to refactor get_or_create some time later.

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

No branches or pull requests

2 participants