[update]Add limit in api/events #742
Conversation
Signed-off-by: “fewdan” <[email protected]>
Signed-off-by: “fewdan” <[email protected]>
Signed-off-by: “fewdan” <[email protected]>
fewdan
requested review from
cwen0,
g1eny0ung,
Gallardot,
YangKeao and
zhouqiang-cl
| @@ -73,27 +71,33 @@ type EventStore interface { | |||
|
|
|||
| // Event represents an event instance. | |||
| type Event struct { | |||
| gorm.Model | |||
There was a problem hiding this comment.
Why gorm.Model is not needed?
There was a problem hiding this comment.
The content in gorm.Model is "ID", "CreatedAt", "UpdatedAt", "DeletedAt". I took out these 4 things directly, so that it is convenient to add the "json" tag.
| @@ -73,27 +71,33 @@ type EventStore interface { | |||
|
|
|||
| // Event represents an event instance. | |||
| type Event struct { | |||
| gorm.Model | |||
| ) | ||
|
|
||
| if filter.LimitStr != "" { |
There was a problem hiding this comment.
The two functions DryListByFilter and ListByFilter has many duplicates because they share the same logic, can we abstract some of them to another function?
There was a problem hiding this comment.
"DryListByFilter" is mainly used by grafana, and may be accessed more frequently than "ListByFilter". In order not to affect "ListByFilter", it is split out. To merge into the same function is not complicated to implement, mainly based on stability considerations.
There was a problem hiding this comment.
I am not saying to combine the two functions into one. But the two share some parts of duplicate code (except the pod records part). Is it okay to pull the duplicate part out and make it another function?
There was a problem hiding this comment.
But this actually is not part of this pr, so we can have an issue for it first and not do it in this pr. It depends on you
There was a problem hiding this comment.
Oh oh, I got it. I just got it wrong. Let me change it in the follow-up pr.
|
/merge |
|
Your auto merge job has been accepted, waiting for:
|
|
/run-all-tests |
* update Signed-off-by: “fewdan” <[email protected]> * update Signed-off-by: “fewdan” <[email protected]> * update Signed-off-by: “fewdan” <[email protected]> Co-authored-by: ti-srebot <[email protected]>
What problem does this PR solve?
Add "limit" in "api/events" to indicate the maximum length of events_list.