advanced-alchemy

advanced-alchemy copied to clipboard

Description

I have recently been bitten by the two following issues, in sequence:

• With repository.get(): in Python, get is one of the most commonly used methods, and it's easy to assume that it means "return something if found, or None (or some other default value) otherwise". In advanced-alchemy's case, it raises an exception when nothing is found.

• With repository.get_one_or_none(), it's easy to assume that it has a similar signature than get. But no, one has to provide the primary key by name (e.g. get(id) v.s get_one_or_none(id=id).). This can also lead to confusion.

In other words, there is a lack of consistency between:

• repository.get() and dict.get()
• repository.get() and repository.get_one_or_none()

It's probably too late to change the API now, but I suggest anyway:

• renaming get to get_one and introducing a get method similar to the current version of get that return None instead of raising a exception.
• Renaming the current get_* methods using a different verb ("fetch"?, "retrieve"? ...)

URL to code causing the issue

No response

MCVE

No response

Steps to reproduce

No response

Screenshots

No response

Logs

No response

Jolt Project Version

0.6.1.

Platform

• [X] Linux
• [ ] Mac
• [ ] Windows
• [ ] Other (Please specify in the description above)

---

Funding

• If you would like to see an issue prioritized, make a pledge towards it!
• We receive the pledge once the issue is completed & verified

@sfermigier thanks for reporting this, and I do see what you mean.

Since we've not hit a stable API yet, I think we still potentially change this by adding some deprecated flags around the original methods.

FWIW, the original intent was to mirror the Django ORM behavior for get. However, now that we have quite a few additional methods maybe we should revisit this.

Let me think about how we might be able to accomplish this.

@sfermigier @jolt-org/maintainers

I've been thinking about this a bit. What do you think about renaming the get method to get_by_id? This should solve the confusion with the get method similarities.

I have since realized that the SQLAlchemy Session object also provides a get method ("Return an instance based on the given primary key identifier, or None if not found.).

So I think get is a pretty reasonable name, as long as it returns None if not found.

get_by_id would still leave the expectation that it behaves like get, so I'm not sure about your proposition.