-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
POC use ent as dbv2 #2940
base: main
Are you sure you want to change the base?
POC use ent as dbv2 #2940
Conversation
Signed-off-by: Wei Zhang <[email protected]>
Signed-off-by: Wei Zhang <[email protected]>
Signed-off-by: Wei Zhang <[email protected]>
I mark it as draft, and we can discuss it here. the PR should actually work for API v2 to get memos. |
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.
Maybe memo_relation
tempList, err := s.Store.ListMemoRelations(ctx, &store.FindMemoRelation{ | ||
MemoID: &request.Id, | ||
}) | ||
tempList, err := s.Store.V2.MemoRelation. |
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.
Should we wrap a method in the store
package?
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.
that's also my first try wrapping the db inside store.
but there are a few concerns, and we can discuss:
- using the origin store interface has no way to distinguish v1 or v2, we can only replace the origin v1 directly in each function step by step.
- it seems not worth creating a new store interface only for ent operations
- the ent client seems to be easy enough to use
Sorry for the late review. Overall, I think it is a good try! |
I have divided the PR into 3 commits to show how ent could be used.