ioxiocom / firedantic

Database models for Firestore using Pydantic base models.
BSD 3-Clause "New" or "Revised" License
43 stars 14 forks source link

Supporting select + order by + limit in queries #43

Closed pjaol closed 10 months ago

pjaol commented 11 months ago

Hi @lietu / @hugovk / @belham / @pfro / @soderluk

I just started using firedantic the other day and I like it a lot, thank you for it ! As I was trying it out, there were a couple of items that I found myself needing and starting wondering if there was a slight change in the design it might make this hugely functional.

Right now it's working similar to mongo with find, find_one, filter but will lag behind firestore client for things like order or select. which I'm implementing by

ClassObj._get_col_ref().select(['date', 'field']).order_by('date').stream()

In firestore this obviously is done by select / order / filter etc.. returning BaseQuery letting you append each method. Might it be worth exploring having multi-inheritance with a BaseQuery which would give you the best of both worlds? e.g. (not tested, probably doesn't work)

class BaseModel(pydantic.BaseModel, BaseQuery, ABC) : 
......
......

class MyModel(BaseModel) : 
....

mymodels = MyModel.select(["fieldA", "fieldB"]).order_by("fieldB").find()

It would cut down on having to keep up or reimplement the clients features and still provide pydantic models to users.

Is this worth looking into?

antont commented 10 months ago

Curious about this too, I'd need a query with order_by with direction=Query.DESCENDING and then limit

lietu commented 10 months ago

Hey @pjaol @antont

Thanks for your interest, but please don't ping random people to try and get attention.

We don't build features based on requests alone, but we are interested in taking a look at PRs if any are submitted when we have the time. At the moment it looks like our schedule for the rest of the year is super busy, but if you make a PR we'll eventually get to it.

pjaol commented 10 months ago

@lietu if you have no interest in working with people, then don't bother open sourcing your code.

lietu commented 10 months ago

@pjaol "no interest in working with people"? I said PRs are welcome, but we're not going to write the code for you. I also asked you not to @ random people, especially as some of the people who you pinged have nothing to do with the project.

lietu commented 10 months ago

Also just so we're clear: "if you have no interest in working with people, then don't bother open sourcing your code" is a very bad and toxic attitude to have. It's open source as in, you're free to use it. It does not mean we're going to fulfill your wishes about it for free. If that's what you think "open source" means, you're going to be very disappointed.

pjaol commented 10 months ago

@lietu nobody is asking you to write code Despite you changing the subject I asked you to create a roadmap, to set direction, open a discussion. I even include how to do it, and have already written a version for myself, I'm trying to get this to a proper open source project so I can contribute it back in a way that works for me and others.

You have a nice handy piece of software, it's useful but it's not complete. The last contributions to the repo before this request was over a year ago, theres no dependency monitoring, no movement. The reason for the broad reach was to determine is this thing still alive.

If it wasn't alive, oh well - I'd remove and do something like marshmallow and wrap pydantic no big deal.

As for my expectations with Open source - you might want to google me before trying to flex toxicity and doing 'your wishes'

And a final word - every input is contribution, every person using your software is validation, people taking time to look at the software is a big deal. The moment you separate it into coders and users and assume you know which one is which is when you start the path of failing.

antont commented 10 months ago

@pjaol have already written a version for myself, I'm trying to get this to a proper open source project so I can contribute it back in a way that works for me and others.

could you make a PR about it then?

as mentioned, I'd have the need for it too.

lietu commented 10 months ago

"Google me", well that's definitely not a sign of a toxic person at all. I think at this point it's fair to say bye @pjaol and please don't bother us again.

lietu commented 10 months ago

@antont if you have the energy to re-create a new issue I see no issue with the requests from you being listed there, but I don't see an obvious way to stop the creator from closing this one.

lietu commented 10 months ago

@antont also as a quick answer to your train of thought while I remember it - Firedantic has been designed to make common simple database actions efficient and easy, without blocking you from writing more advanced code yourself.

You can always e.g. extend the base classes, or call MyModel._get_col_ref() and do the advanced queries that you need like that. One of the reasons why Firedantic (and similar libraries we've also worked on arangodantic and mongodantic) is so nice to work with is because it does not even attempt to support every potential feature of the underlying DB layer / connector, as they wouldn't all make sense in this mode of operation - e.g. selecting only a few fields when interacting with classes that are supposed to represent the entire object.

So if you feel like there is something Firedantic doesn't excel at, you shouldn't necessarily feel it won't work for you - it just won't solve everything for you, but it should still collaborate with your more custom logic reasonably easily.

antont commented 10 months ago

@lietu MyModel._get_col_ref() and do the advanced queries that you need like that.

Just did that 5 minutes ago, happy that it works :)

In this case it was for modifying a value inside a JSON field in the doc, so a different thing, that might actually be nice to support. I'll consider opening a separate issue about that.

And yep your rationale is good. Anyway there are similar libs that support things like order_by and limit so might make sense to have those.

lietu commented 10 months ago

Yeah frankly order_by and limit should be incredibly easy to support by just adding optional keyword arguments to the find() with a little bit of logic inside. No need to reinvent the API for that.

antont commented 9 months ago

Yeah frankly order_by and limit should be incredibly easy to support by just adding optional keyword arguments to the find() with a little bit of logic inside.

I added those, have been using happily for some time, and put a PR here too, https://github.com/ioxiocom/firedantic/pull/51