Update session history - #523
Conversation
…ate-sessions-history # Conflicts: # core/connection/session_storage.go
…ate-sessions-history # Conflicts: # core/connection/manager_test.go # core/connection/session_storage.go
| Duration: se.Duration, | ||
| BytesSent: uint64(se.DataStats.BytesSent), | ||
| BytesReceived: uint64(se.DataStats.BytesReceived), | ||
| Duration: getDuration(se.TimeStarted, se.TimeUpdated), |
There was a problem hiding this comment.
move the getDuration method to connection.Session struct, that way you can just call se.GetDuration()
| func (repo *SessionStorage) Update(sessionID session.ID, TimeUpdated time.Time, dataStats stats.SessionStats) error { | ||
| // update two fields by sessionID | ||
| se := Session{SessionID: sessionID, Duration: duration, DataStats: dataStats} | ||
| se := Session{SessionID: sessionID, TimeUpdated: TimeUpdated, DataStats: dataStats} |
There was a problem hiding this comment.
Do we save session status - NEW, IN Progress, COMPLETED ? How can I tell which session is finished which is ongoing?
There was a problem hiding this comment.
Is there a need for session status in history list? This can be added as feature later as we get feedback from using this endpoint
There was a problem hiding this comment.
imho NEW and COMPLETED should be enough. New session will have TimeStarted set, completed session will have TimeUpdated set?
There was a problem hiding this comment.
added statuses New and Completed
| @@ -127,6 +127,19 @@ func consumerConnectFlow(t *testing.T, tequilapi *tequilapi_client.Client, consu | |||
| assert.NoError(t, err) | |||
| seelog.Info("Shifted consumer IP: ", vpnIp) | |||
There was a problem hiding this comment.
| seelog.Info("Shifted consumer IP: ", vpnIp) | |
| seelog.Info("Changed consumer IP: ", vpnIp) |
| } | ||
|
|
||
| // GetSessions returns all sessions from history | ||
| func (client *Client) GetSessions() (endpoints.SessionsDTO, error) { |
There was a problem hiding this comment.
sessions := endpoints.SessionsDTO{}
and using only sessions should be simpler.
| Duration int // in seconds | ||
| DataStats stats.SessionStats | ||
| Status SessionStatus | ||
| TimeUpdated time.Time // is updated on disconnect event |
There was a problem hiding this comment.
If it updated on disconnect event, maybe we should call it Finished?
There was a problem hiding this comment.
Later Session object can be updated during session. So I suggest to leave more generic name
| ProviderID identity.Identity | ||
| ServiceType string | ||
| ProviderCountry string | ||
| TimeStarted time.Time |
There was a problem hiding this comment.
Maybe we can call it just Started and Finished, it has time time.Time no need Time word at the name of the field.
| // GetDuration returns delta in seconds (TimeUpdated - TimeStarted) | ||
| func (se *Session) GetDuration() uint64 { | ||
| if se.TimeUpdated.IsZero() { | ||
| return 0 |
There was a problem hiding this comment.
If a session is not finished, GetDuration can return time.Now() - se.TimeStarted instead of 0.
There was a problem hiding this comment.
If a session is not finished, that can mean a case where app crashes. So in that case time.Now() - se.TimeStarted will provide misleading result.
In current logic, in that case it will provide value 0.
| } | ||
|
|
||
| // GetStatus converts status constant to string | ||
| func (se *Session) GetStatus() string { |
There was a problem hiding this comment.
I think it should be String() method for the SessionStatus type.
| // update two fields by sessionID | ||
| se := Session{SessionID: sessionID, Duration: duration, DataStats: dataStats} | ||
| func (repo *SessionStorage) Update(sessionID session.ID, TimeUpdated time.Time, dataStats stats.SessionStats, | ||
| status SessionStatus) error { |
There was a problem hiding this comment.
How does this formatted, why it splitted to 2 lines?
…-sessions-history
…ysteriumNetwork/node into update-sessions-history
5 participants
Closes #433