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

Extract RPC summaries from backend responses. #158

Merged
merged 13 commits into from
Nov 5, 2019

Conversation

jkohen
Copy link

@jkohen jkohen commented Oct 30, 2019

Implemented CreateTimeSeriesSummary parser. A follow-up change will use this to produce metrics.

@jkohen jkohen requested a review from a team October 30, 2019 16:25
@jkohen jkohen self-assigned this Oct 30, 2019
@ghost ghost requested review from bmoyles0117 and removed request for a team October 30, 2019 16:25
Copy link
Member

@igorpeshansky igorpeshansky left a comment

Choose a reason for hiding this comment

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

You may have to rebase your branch on the latest stackdriver-agent-5.8.1 — it's still not quite stable, and underwent another rebase. It should be simple in an interactive rebase: just delete 6a0aef8 and 8a2a4f8, and replay the rest of the commits.

@jkohen
Copy link
Author

jkohen commented Oct 30, 2019

You may have to rebase your branch on the latest stackdriver-agent-5.8.1 — it's still not quite stable, and underwent another rebase. It should be simple in an interactive rebase: just delete 6a0aef8 and 8a2a4f8, and replay the rest of the commits.

Thanks for the warning. Can you provide the commands I need to run to do the rebase? Any idea when it'll become stable so that a simple git merge works?

@igorpeshansky
Copy link
Member

I was suggesting an interactive rebase:

git fetch && git checkout stackdriver-agent-5.8.1 && git reset --hard origin/stackdriver-agent-5.8.1
git checkout rpc_response
git rebase -i stackdriver-agent-5.8.1
# In the rebase-todo, drop the first two commits with numbers 6a0aef8 and 8a2a4f8 and save.
# You should have a clean rebase at this point. Check it with:
git log rpc_response..stackdriver-agent-5.8.1  # should be empty

@qingling128 and I just agreed to freeze the current stackdriver-agent-5.8.1 branch history, so this was the last rebase.

@bmoyles0117
Copy link

I'll wait for the rebase to review, apparently the DIFF shows changes that are not part of your changes. When you've done the rebase, make sure you do git push -f to force the push.

@jkohen
Copy link
Author

jkohen commented Oct 30, 2019

I was suggesting an interactive rebase:

git fetch && git checkout stackdriver-agent-5.8.1 && git reset --hard origin/stackdriver-agent-5.8.1
git checkout rpc_response
git rebase -i stackdriver-agent-5.8.1
# In the rebase-todo, drop the first two commits with numbers 6a0aef8 and 8a2a4f8 and save.
# You should have a clean rebase at this point. Check it with:
git log rpc_response..stackdriver-agent-5.8.1  # should be empty

@qingling128 and I just agreed to freeze the current stackdriver-agent-5.8.1 branch history, so this was the last rebase.

Done, thank you. I didn't see 8a2a4f8 though.

@qingling128
Copy link

This one seems ready for review. @bmoyles0117 @igorpeshansky

@bmoyles0117
Copy link

Reviewing now, thanks for the ping @qingling128

Copy link

@bmoyles0117 bmoyles0117 left a comment

Choose a reason for hiding this comment

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

LGTM, barring 2 small comments.

src/collectd_time_series_response_test.json Outdated Show resolved Hide resolved
src/utils_stackdriver_json.c Outdated Show resolved Hide resolved
@jkohen
Copy link
Author

jkohen commented Nov 5, 2019

Submitting. Thanks, Bryan!

@jkohen jkohen merged commit e83cca7 into stackdriver-agent-5.8.1 Nov 5, 2019
@jkohen jkohen deleted the rpc_response branch November 5, 2019 15:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants