-
Couldn't load subscription status.
- Fork 40
[wip] session: impl database session #539
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
base: main
Are you sure you want to change the base?
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #539 +/- ##
===================================================
- Coverage 74.83858% 74.81446% -0.02412%
===================================================
Files 195 201 +6
Lines 29891 31260 +1369
===================================================
+ Hits 22370 23387 +1017
- Misses 6339 6734 +395
+ Partials 1182 1139 -43
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| "time" | ||
|
|
||
| "gorm.io/driver/mysql" | ||
| "gorm.io/gorm" |
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.
这里用gorm是不是有点重了。 可以考虑用内置的包?
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.
主要是期望能直接适配pg、mysql、clickhouse
| // | ||
|
|
||
| // Package database provides the Database instance info management. | ||
| package database |
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.
这个包名会不会太大了,要不就叫 gorm 怎样
| key.AppName, key.UserID, key.SessionID). | ||
| Order("timestamp ASC"). | ||
| Limit(int(count - int64(s.opts.sessionEventLimit))). | ||
| Delete(&sessionEventModel{}).Error; err != nil { |
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.
这里要不要考虑增加一个软删除的配置, 获取的时候加过期条件判断。如果配置了软删除选项,这里就别Delete了。 业务这边应该也有希望保留全部历史Events的诉求, 用于做一些事后的分析,效果评估的素材之类的
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.
这个可以的,我加一下
No description provided.