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.

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

Add Group support to the Monitoring API #2035

Merged
merged 8 commits into from
Aug 4, 2016
Merged

Add Group support to the Monitoring API #2035

merged 8 commits into from
Aug 4, 2016

Conversation

supriyagarg
Copy link
Contributor

This change allows users to both read and write Stackdriver groups. It allows users to:

  • fetch groups
  • list groups
  • list a group's ancestors, descendants, and children groups
  • create, update, and delete groups.
  • list a group's members

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jul 29, 2016
@supriyagarg supriyagarg changed the title Add support for Groups API under Monitoring Add Group support to the Monitoring API Jul 29, 2016
@tseaver tseaver added the api: monitoring Issues related to the Cloud Monitoring API. label Jul 29, 2016
jgeewax
jgeewax reviewed Jul 30, 2016
View reviewed changes
docs/monitoring-usage.rst

You can get a specific group based on it's ID as follows::

>>> group = client.fetch_group('1001')

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

dhermes
dhermes reviewed Aug 1, 2016
View reviewed changes
gcloud/monitoring/group.py
params['interval.endTime'] = _format_timestamp(end_time)

if start_time is not None:
params['interval.startTime'] = _format_timestamp(start_time)

This comment was marked as spam.

Sign in to view

@dhermes
Copy link
Contributor

dhermes commented Aug 1, 2016

OK I made it to the bottom. Review finished.

@supriyagarg
Copy link
Contributor Author

Thanks for the comments @dhermes and @jgeewax. I have replied to some of the comments - and plan to address the rest in a follow up commit.

An outstanding question - since Client class under gcloud.monitoring already uses the verb fetch to get objects via API calls, the fetch_group method does the same. Is it okay to keep it as is? Mixing two verbs in the same sub-component will be confusing for users.

This naming convention will also apply to the method parent which I will rename to fetch_parent.

dhermes
dhermes reviewed Aug 2, 2016
View reviewed changes
gcloud/monitoring/group.py
self.filter = filter_string
self.is_cluster = is_cluster

if group_id:

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@supriyagarg
Copy link
Contributor Author

I just submitted commits to address the outstanding comments.

@dhermes
Copy link
Contributor

dhermes commented Aug 3, 2016

@supriyagarg I think all concerns have been addressed right?

Before merging, can you clean up the commits? In particular Merge branch 'master' into monitoring-api is just icky to have merged into master. (I can lend a hand if need be.)

@supriyagarg
Add Group support to the Monitoring API
cc0f814 
Users can now fetch and list Stackdriver groups, and list a group's
members.
@supriyagarg
Added support for creating and deleting Stackdriver groups.
0018b01 
Also allow a user to specify either the group ID or name to initilize
it. The client class now has a factory constructor groups.
@supriyagarg
Added support for updating Stackdriver groups.
335d909
@supriyagarg
Add system tests for the groups API under monitoring.
cedca63
@supriyagarg
Change Groups API to take group and parent IDs as the inputs.
c795598 
The fully qualified names are set by the class based on client.project.
Also, the property Group.id can only be set during initialization now.
@supriyagarg
Remove the helpers module from monitoring API.
3b3a996 
This commit makes the code consistent with the update to
the helper function _datetime_to_rfc3339.
@supriyagarg
Edits to the Groups API
d205cec 
* Renamed some of the methods of gcloud.monitoring.Groups that get data
  from the API:
  * parent -> fetch_parent
  * members -> list_members
  * children -> list_children
  * ancestors -> list_ancestors
  * descendants -> list_descendants

* Renamed / deleted some helper methods
  * group_id_from_name -> _group_id_from_name
  * _group_name_from_id -> _group_name_from_id
  * _init_from_dict -> _set_properties_from_dict
  * delete path_helper

* Minimized the payload while checking if a group exists.

* Updated the docstrings to be more consistent. Especially added
  examples where the group ID is not an integer.
@supriyagarg
Remove retries during system tests for group.
5dfa1be
@supriyagarg
Copy link
Contributor Author

supriyagarg commented Aug 4, 2016 •
edited
Loading

@dhermes: That is correct. I just cleaned up the commits to remove the Merge branch 'master' commits. Let me know if you need me to edit anything else.

@dhermes dhermes merged commit fc0bc0e into googleapis:master Aug 4, 2016
@supriyagarg supriyagarg deleted the monitoring-api branch August 4, 2016 13:29
@dhermes dhermes mentioned this pull request Sep 19, 2016
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
api: monitoring Issues related to the Cloud Monitoring API. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@supriyagarg @dhermes @jgeewax @tseaver @rimey @googlebot