VForWaTer / metacatalog

Modular metadata management platform for environmental data.
https://vforwater.github.io/metacatalog
GNU General Public License v3.0
3 stars 1 forks source link

`Entry.from_dict` has no real purpose #251

Open mmaelicke opened 1 year ago

mmaelicke commented 1 year ago

RIght now, the Entry.from_dict method is actually creating a new entry in the database. I don't think that we actually need this, except for import/export. In my opinion it's the better implementation to deprecate the behavior of from_dict and finally, it should return a Entry instance, that has not yet been committed to the database. This way we can implement a loading logic, that lazy-loads an Entry with all information available in the database. If we need this import-from-dict functionality, all we need in a new function is add a session.add(Entry) and session.commit() command to the result of from_dict.

@AlexDo1 what do you think? This is completely up to discussion.

Todos

AlexDo1 commented 1 year ago

I also cannot see the case where we would need the current implementation of the from_dict method, so its fine to deprecate it.

I like that the Entry instance would still be created, but not commited to the database. So we don't really lose any functionality of the from_dict method but a user has the additional possibilty to check the created Entry instance before commiting it to the database.

mmaelicke commented 1 year ago

I also cannot see the case where we would need the current implementation of the from_dict method, so its fine to deprecate it.

I like that the Entry instance would still be created, but not commited to the database. So we don't really lose any functionality of the from_dict method but a user has the additional possibilty to check the created Entry instance before commiting it to the database.

Then I turn this issue into a model-tagged development issue and self-assign it.