You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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
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
The text was updated successfully, but these errors were encountered:
@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.
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.
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 thanget
. But no, one has to provide the primary key by name (e.g.get(id)
v.sget_one_or_none(id=id)
.). This can also lead to confusion.In other words, there is a lack of consistency between:
repository.get()
anddict.get()
repository.get()
andrepository.get_one_or_none()
It's probably too late to change the API now, but I suggest anyway:
get
toget_one
and introducing aget
method similar to the current version ofget
that return None instead of raising a exception.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
Funding
The text was updated successfully, but these errors were encountered: