Skip to content
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

MYST-182 Discovery. Use new stats endpoint #77

Merged
merged 2 commits into from Jan 2, 2018

Conversation

Projects
None yet
4 participants
@donce
Copy link
Contributor

commented Dec 29, 2017

Should be merged only after mysteriumnetwork/api#11.

response, err := client.doPostRequest("client_send_stats", sessionStats)
func (client *clientRest) CreateSessionStats(sessionId string, sessionStats dto.SessionStats) (err error) {
path := fmt.Sprintf("sessions/%s/stats", sessionId)
response, err := client.doPostRequest(path, sessionStats)

This comment has been minimized.

Copy link
@shroomist

shroomist Dec 29, 2017

Contributor

function name misleading - this doesn't create but does send the stats

This comment has been minimized.

Copy link
@tadovas

tadovas Dec 29, 2017

Member

Agreed.

This comment has been minimized.

Copy link
@donce

donce Dec 29, 2017

Author Contributor

Done! 🍭

@shroomist

This comment has been minimized.

Copy link
Contributor

commented Dec 29, 2017

Please use conventional PR Titles. in this case might be:
MYST-182 Discovery. Use new stats endpoint

@donce donce changed the title Use new stats endpoint MYST-182 Use new stats endpoint Dec 29, 2017

@donce donce changed the title MYST-182 Use new stats endpoint MYST-182 Discovery. Use new stats endpoint Dec 29, 2017

@tadovas
Copy link
Member

left a comment

LGTM

@Waldz

Waldz approved these changes Jan 2, 2018

@@ -1,6 +1,6 @@
package dto

type NodeStatsRequest struct {
NodeKey string `json:"node_key"`
Sessions []SessionStats `json:"sessions"`

This comment has been minimized.

Copy link
@Waldz

Waldz Jan 2, 2018

Member

You can remove NodeStatsRequest.Sessions fields at all

This comment has been minimized.

Copy link
@donce

donce Jan 2, 2018

Author Contributor

It's currently being used and is required by api. It could be removed from everywhere, since it's empty array at the moment, but it will require changing api as well, and might be worth to do this in https://mysteriumnetwork.atlassian.net/browse/MYST-92

@donce donce merged commit d8d469a into master Jan 2, 2018

@donce donce deleted the feature/MYST-182-use-new-stats-endpoint branch Jan 2, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.