Update monitoring gapic client, re-integrate helpers by supriyagarg · Pull Request #4785 · googleapis/google-cloud-python

supriyagarg · 2018-01-24T23:43:11Z

Added back support for the Query class, and for converting timeseries data into a pandas DataFrame object.

Most of the code was ported over from the master branch, and updated to use the autogenerated code. Also, add utf-8 encoding to proto/common_pb2.py.

theacodes · 2018-01-26T20:20:36Z

@supriyagarg I don't see the actual addition of the monitoring gapic client here?

lukesneeringer

This is mostly good; I have a couple items cited that I believe could/should be improved.

I also want to thank you for excellent commenting.

Sign in to view

+ :rtype: :class:`pandas.DataFrame`
+ :returns: A dataframe where each column represents one time series.
+ """
+ import pandas # pylint: disable=import-error


Sign in to view

+ """
+ import pandas # pylint: disable=import-error
+
+ if labels is not None:


Sign in to view

+ headers.append(_extract_header(time_series))
+
+ # Implement a smart default of using all available labels.
+ if label is None and labels is None:


Sign in to view

+ # Build a multi-level stack of column headers. Some labels may
+ # be undefined for some time series.
+ levels = []
+ for key in labels or [label]:


Sign in to view

+ # by specifying "label".
+ dataframe.columns = pandas.MultiIndex.from_arrays(
+ levels,
+ names=labels or None)


Sign in to view

+ """
+ # Using copy.deepcopy() would be appropriate, except that we want
+ # to copy self._client only as a reference.
+ new_query = copy.copy(self)


lukesneeringer · 2018-01-31T15:33:20Z

@supriyagarg I don't see the actual addition of the monitoring gapic client here?

I added it in the base branch.

supriyagarg · 2018-01-31T23:25:56Z

@lukesneeringer: Thanks very much for your comments. I have addressed all of them.
The code is largely copied over from the master branch, including the comments - this was originally written in collaboration with @rimey

Also, populat the 'aggregation' field is now populated only if at least one of its components has been set.

chemelnucfin · 2018-02-01T08:20:13Z

@lukesneeringer How does this tie in with autogen? Is just specifying automodule will autogenerate that class? And then query and _dataframe are just manual classes?

supriyagarg requested a review from lukesneeringer as a code owner January 24, 2018 23:43

googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jan 24, 2018

supriyagarg force-pushed the monitoring-autogen branch 2 times, most recently from 69759c8 to 84eb2eaCompare January 25, 2018 00:18

supriyagarg added 2 commits January 24, 2018 19:52

Add support for the Query class, and creating Pandas DataFrame.
c874ef7
Most of the code was ported over from the master branch, and updated to use the autogenerated code. Also, add utf-8 encoding to proto/common_pb2.py.

Update the doc files for monitoring to reflect the autogen code.
dfc929f

supriyagarg force-pushed the monitoring-autogen branch from 84eb2ea to dfc929fCompare January 25, 2018 00:53

tseaver added the api: monitoring Issues related to the Cloud Monitoring API. label Jan 25, 2018

chemelnucfin added the type: process A process-related concern. May include testing, release, or the like. label Jan 25, 2018

chemelnucfin assigned waprin, chemelnucfin and supriyagarg Jan 26, 2018

lukesneeringer reviewed Jan 31, 2018
View reviewed changes

supriyagarg force-pushed the monitoring-autogen branch from e0a242a to 36fc192Compare January 31, 2018 22:35

Address comments on _dataframe.py
09a8f1d

supriyagarg force-pushed the monitoring-autogen branch from 36fc192 to 09a8f1dCompare January 31, 2018 22:41

Update the copy method to __deepcopy__()
18b9e37
Also, populat the 'aggregation' field is now populated only if at least one of its components has been set.

supriyagarg force-pushed the monitoring-autogen branch from 9636908 to 18b9e37Compare January 31, 2018 23:30

Add unit tests for the manual code in Monitoring.
bc3f82f

supriyagarg force-pushed the monitoring-autogen branch from bd09c12 to bc3f82fCompare February 1, 2018 19:13

theacodes changed the title ~~Monitoring autogen~~Update monitoring gapic client, re-integrate helpersApr 13, 2018

theacodes unassigned chemelnucfin Apr 13, 2018

theacodes merged commit 10643a8 into googleapis:monitoring-autogenApr 13, 2018

theacodes pushed a commit that referenced this pull request Apr 19, 2018
Add support for the Query class and Pandas dataframes (#4785)
d979cac

supriyagarg deleted the monitoring-autogen branch April 20, 2018 05:03

theacodes unassigned waprin Sep 28, 2018

Uh oh!

Conversation

supriyagarg commented Jan 24, 2018

Uh oh!

theacodes commented Jan 26, 2018

Uh oh!

lukesneeringer left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

lukesneeringer commented Jan 31, 2018

Uh oh!

supriyagarg commented Jan 31, 2018

Uh oh!

chemelnucfin commented Feb 1, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants