Skip to content

Conversation

@vmarmol
Copy link
Contributor

@vmarmol vmarmol commented Dec 9, 2014

No description provided.

@vmarmol
Copy link
Contributor Author

vmarmol commented Dec 9, 2014

Ping @vishh

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not return immediately if there is an error?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only reason is to provide partial stats if they're available (i.e.: only CPU and memory)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm. That would complicate the API. If we intend to ignore network errors, why not just log it instead of propagating it?

@vmarmol
Copy link
Contributor Author

vmarmol commented Dec 10, 2014

Done, PTAL.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/insped/inspect?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two arguments passed, but only is expected.

@vishh
Copy link
Contributor

vishh commented Dec 10, 2014

@vmarmol: Just a few more comments.

@vmarmol
Copy link
Contributor Author

vmarmol commented Dec 10, 2014

Ooops, committed more than I meant to. Fixed, PTAL.

vishh added a commit that referenced this pull request Dec 10, 2014
Report error while fetching network stats.
@vishh vishh merged commit 5064ad4 into google:master Dec 10, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants