Skip to content

Containerized PCP Integration #1986

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jan 6, 2021
Merged

Containerized PCP Integration #1986

merged 1 commit into from
Jan 6, 2021

Conversation

@Maxusmusti
Copy link
Member

@Maxusmusti Maxusmusti commented Nov 17, 2020 •
edited
Loading

Now working with local and remote hosts!

Unrelated to PR:

  • add pmproxy to pcp-collector image

@Maxusmusti Maxusmusti self-assigned this Nov 17, 2020
@portante portante added enhancement tools Of and related to the operation and behavior of various tools (iostat, sar, etc.) labels Nov 18, 2020
@portante portante added this to the v0.71 milestone Nov 18, 2020
@portante
Copy link
Member

portante commented Nov 18, 2020

Replaces and updated PR #1759.

This was referenced Nov 18, 2020
Closed
Closed
portante
portante reviewed Nov 20, 2020
View reviewed changes
Copy link
Member

@portante portante left a comment

Choose a reason for hiding this comment

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

Coming along nicely.

@Maxusmusti Maxusmusti force-pushed the pcp-redo branch 2 times, most recently from 6471600 to f23f0a1 Compare December 4, 2020 02:15
@lgtm-com
Copy link

lgtm-com bot commented Dec 4, 2020

This pull request introduces 1 alert when merging 957109c into 4b4a721 - view on LGTM.com

new alerts:

  • 1 for Unused local variable

@lgtm-com
Copy link

lgtm-com bot commented Dec 8, 2020

This pull request introduces 1 alert when merging 853d7e9 into dcb6ed2 - view on LGTM.com

new alerts:

  • 1 for Syntax error

@Maxusmusti Maxusmusti marked this pull request as ready for review December 9, 2020 08:20
portante
portante requested changes Dec 9, 2020
View reviewed changes
Copy link
Member

@portante portante left a comment

Choose a reason for hiding this comment

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

First pass review, more to come. Can you squash the set of commits down a bit? 22 seems to be more than we need to review. =)

@Maxusmusti Maxusmusti requested a review from portante December 10, 2020 20:03
portante
portante requested changes Dec 10, 2020
View reviewed changes
Copy link
Member

@portante portante left a comment

Choose a reason for hiding this comment

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

Still reviewing ...

@Maxusmusti Maxusmusti requested a review from portante December 12, 2020 04:38
portante
portante requested changes Dec 12, 2020
View reviewed changes
Copy link
Member

@portante portante left a comment

Choose a reason for hiding this comment

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

Looks like you also need to remove agent/tool-scripts/postprocess/pcp-postprocess.

@Maxusmusti Maxusmusti requested a review from portante December 16, 2020 12:19
portante
portante requested changes Dec 16, 2020
View reviewed changes
Copy link
Member

@portante portante left a comment

Choose a reason for hiding this comment

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

A few more comments, and it looks like you need to "lint" the code changes.

dbutenhof
dbutenhof reviewed Dec 17, 2020
View reviewed changes
@Maxusmusti Maxusmusti force-pushed the pcp-redo branch 3 times, most recently from 2ded23a to 33761df Compare January 5, 2021 22:34
@Maxusmusti
Copy link
Member Author

Maxusmusti commented Jan 5, 2021

Fully tested with local+remote nodes

@Maxusmusti Maxusmusti requested a review from portante January 5, 2021 22:58
portante
portante requested changes Jan 6, 2021
View reviewed changes
Copy link
Member

@portante portante left a comment

Choose a reason for hiding this comment

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

Can you squash down to one commit?

@Maxusmusti Maxusmusti requested a review from portante January 6, 2021 00:28
@Maxusmusti
Copy link
Member Author

Maxusmusti commented Jan 6, 2021

All in one now

portante
portante previously approved these changes Jan 6, 2021
View reviewed changes
dbutenhof
dbutenhof reviewed Jan 6, 2021
View reviewed changes
dbutenhof
dbutenhof previously approved these changes Jan 6, 2021
View reviewed changes
@dbutenhof
Copy link
Member

dbutenhof commented Jan 6, 2021

And yet ... needs a rebase.

portante
portante requested changes Jan 6, 2021
View reviewed changes
Copy link
Member

@portante portante left a comment

Choose a reason for hiding this comment

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

@Maxusmusti, one more rebase and we can merge.

@portante portante merged commit 16f5522 into distributed-system-analysis:master Jan 6, 2021
@portante portante mentioned this pull request Jan 11, 2021
Merged
@portante portante mentioned this pull request Feb 24, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@portante portante portante approved these changes

@dbutenhof dbutenhof dbutenhof approved these changes

@ndokos ndokos Awaiting requested review from ndokos

Assignees

@Maxusmusti Maxusmusti

Labels

Agent enhancement tools Of and related to the operation and behavior of various tools (iostat, sar, etc.)

Projects

None yet

Milestone

v0.71

Development

Successfully merging this pull request may close these issues.

4 participants

@Maxusmusti @portante @dbutenhof @not4win