Update monitoring gapic client, re-integrate helpers #4785

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

theacodes merged 5 commits into googleapis:monitoring-autogen from supriyagarg:monitoring-autogen

Apr 13, 2018

Contributor

supriyagarg commented Jan 24, 2018

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

supriyagarg requested a review from lukesneeringer as a code owner

January 24, 2018 23:43

googlebot added the cla: yes label

supriyagarg force-pushed the monitoring-autogen branch 2 times, most recently from 69759c8 to 84eb2ea Compare

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 dfc929f Compare

January 25, 2018 00:53

tseaver added the api: monitoring label

chemelnucfin added the type: process label

chemelnucfin assigned waprin, chemelnucfin and supriyagarg

Contributor

theacodes commented Jan 26, 2018

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

lukesneeringer reviewed

View reviewed changes

Contributor

lukesneeringer left a comment

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.

monitoring/google/cloud/monitoring/_dataframe.py Outdated

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

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

monitoring/google/cloud/monitoring/_dataframe.py Outdated

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

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

monitoring/google/cloud/monitoring/_dataframe.py Outdated

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

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

monitoring/google/cloud/monitoring/_dataframe.py Outdated

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

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

monitoring/google/cloud/monitoring/_dataframe.py Outdated

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

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

monitoring/google/cloud/monitoring/query.py

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

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

Contributor

lukesneeringer commented Jan 31, 2018

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

I added it in the base branch.

supriyagarg force-pushed the monitoring-autogen branch from e0a242a to 36fc192 Compare

January 31, 2018 22:35


          Address comments on _dataframe.py

09a8f1d

supriyagarg force-pushed the monitoring-autogen branch from 36fc192 to 09a8f1d Compare

January 31, 2018 22:41

Contributor Author

supriyagarg commented Jan 31, 2018

@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


          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 18b9e37 Compare

January 31, 2018 23:30

Contributor

chemelnucfin commented Feb 1, 2018

@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?


          Add unit tests for the manual code in Monitoring.

bc3f82f

supriyagarg force-pushed the monitoring-autogen branch from bd09c12 to bc3f82f Compare

February 1, 2018 19:13

theacodes changed the title ~~Monitoring autogen~~ Update monitoring gapic client, re-integrate helpers

theacodes unassigned chemelnucfin

theacodes merged commit 10643a8 into googleapis:monitoring-autogen

theacodes pushed a commit that referenced this pull request


          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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: monitoring cla: yes type: process