-
Notifications
You must be signed in to change notification settings - Fork 25
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
Other missing SQLAlchemy features #79
Comments
Consider also:
|
Good ones! |
On the other hand, I love how extendable FastCRUD class is. I was able to easily override So while flexible FastCRUD is great and highly boosts flexibility of the EndpointCreater, at some point I started wondering if it would be good to incentivize users to override That's probably very conceptual discussion for far future, as the CRUD part is the focus for now, but I'm curious about your thoughts. |
Totally agree with you, @JakNowy, I think the goal is: if a solution is more complex than just overriding |
SQLAlchemy2 provides comprehensive list of supported operators. We might pick those which we consider worth supporting. I could volunteer on working on that, I'm just not sure how we would like to tackle cases like distinct() and group_by(). For the endpoint creator, I've issued a PR. It's just a general demonstration, not finished yet, I'm looking forward to hearing your thoughts. I could finalize that after addressing the issue of SQLAlchemy operators and getting aligned on what you'd like to see. |
Sounds good to me. We may leave the harder stuff aside until it's necessary, maybe by then we'll have a clearer path ahead.
To be honest, I'm not sure I understood exactly where it's headed. Is the Idea for |
Exactly, that's the idea. Let's say in 75% cases generic CRUD will be sufficient. In other 25% cases, user should be able to override the statement (eg. to perform some joins or complex filters). To be honest I thought all the improvements we make in the FastCRUD also apply to EndpointCreator and I was surprised to see that we only support generic |
Also the concept of my |
That is the idea eventually, but I'm prioritizing improving
I think it's a good way to overcome while we're not applying this stuff do |
We could if we wanted to apply a filter or something like it to all endpoints at once. |
Sounds reasonable to me. I think that's something for you to think about and make a final design decision. When it comes to this issue, I'm going to implement it this week, also including |
Sure. I think the class EndpointCreator:
def __init__(self, model, db_session: AsyncSession, query_configs=None):
self.model = model
self.db_session = db_session
self.query_configs = query_configs or {}
def default_read_item_query(self):
return select(self.model)
def default_read_item_query(self):
return self.default_base_query()
def default_create_item_query(self):
return self.default_base_query()
def default_update_item_query(self):
return self.default_base_query()
def default_delete_item_query(self):
return self.default_base_query()
def get_base_query(self, operation_type):
if operation_type in self.query_configs:
return self.query_configs[operation_type]
else:
return getattr(self, f"default_{operation_type}_query")()
def _read_item(self):
"""Creates an endpoint for reading a single item from the database."""
@apply_model_pk(**self._primary_keys_types)
async def endpoint(db: AsyncSession = Depends(self.session), **pkeys):
base_query = self.get_base_query('read_item')
for key, value in pkeys.items():
base_query = base_query.where(getattr(self.model, key) == value)
result = await db.execute(base_query)
item = result.scalars().first()
if not item: # pragma: no cover
raise NotFoundException(detail="Item not found")
return item # pragma: no cover
return endpoint Maybe something like this. Btw, this would close #41. |
There are a few sql things that are relevant and are currently missing from
FastCRUD
, I'll list some of them and you guys may add what I forgetdistinct
clausegroup by
like
,ilike
,between
operatorsis_
andisnot_
operatorsThe text was updated successfully, but these errors were encountered: