-
Notifications
You must be signed in to change notification settings - Fork 61
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
add beanie support #185
base: main
Are you sure you want to change the base?
add beanie support #185
Conversation
Thank you for submitting the PR! I appreciate your effort and initiative. I'll do my best to review your changes as soon as possible. In the meantime, you can refer to the test cases for 'odmantic' and 'mongoengine' for testing guidance. With a quick look, I noticed some comments are in Spanish. To ensure clarity for everyone, it would be better to have them in English. Could you please update those comments? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello, thank you again for your PR. After reviewing your code, I have a few suggestions and considerations that we need to address before merging:
-
Flexibility:
- It should be possible to run the basic example with only
starlette-admin
andbeanie
as dependencies. - Do we really need
mongoengine
as a dependency to usebeanie
? - Do we really need
fastapi
as a dependency? I think, If the code works with starlette, the support for fastapi should be straightforward - We should make
Gridfs
optional. - To improve extensibility, consider putting the logic of file and image creation inside the File and Image classes, making it easier to override and customize.
- It should be possible to run the basic example with only
-
Converters:
Version 0.9.0 introduces a new approach for model conversion (Enhance fields conversion logic to support custom converters #191).I believe the converter forBeanie
is similar toODMantic
as they are both based mostly on Python type annotation. You can refer to ODMantic implementation here -
ExpressionField:
It is important to support fields lists such as[User.id, User.name]
. Take a look at ExpressionField -
QueryBuilder:
The current query builder is not functioning properly. I suggest using ExpressionField to build the queries and letting Beanie handle the conversion to the corresponding database query. For example, using an expression likeUser.age > 90
should generate the appropriate database query.
Thanks,
Would love to see this getting merged. Any update on this? |
Hello @opaniagu , |
Hello, I have little time at the moment. The main problem is that for me (I think) it is already a functional version, but when I was finishing you changed the core and it means I have to review everything from scratch. |
I truly appreciate your effort, and I'm sorry if the changes inadvertently affected your work. The intention was to improve the conversion process and provide customization options. |
@opaniagu Any update on this? This PR fix and merge could really help alot. |
Hey @opaniagu, I'd like to see this PR to be merged but as far as I can see, the core is outdated. Do you plan to work on this? |
Hi, really now I haven´t time to continue, maybe someone can take it, and finisih. |
I see, thank you for trying it out 🙏. It might give people some idea. I'm going to play with Beanie in the future. It might be useful. |
I share this PR to be able to support 'beanie' https://beanie-odm.dev/.
Please send feedback and help me test.
Also, GridFs is supported.