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

Allow using a metric type in the Query method select_metrics. #2735

Closed
wants to merge 2 commits into from
Closed

Allow using a metric type in the Query method select_metrics. #2735

wants to merge 2 commits into from

Conversation

supriyagarg
Copy link
Contributor

This is useful when a user wants to apply the exact same filters (duration, aggregation, resource labels) for a different metric type.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Nov 14, 2016
@supriyagarg
Copy link
Contributor Author

@rimey: Please take a look.

@daspecster daspecster added the api: monitoring Issues related to the Cloud Monitoring API. label Nov 14, 2016
@supriyagarg supriyagarg force-pushed the monitoring_select_metrics branch 4 times, most recently from 4d42b87 to 90ff89b Compare November 14, 2016 23:29
dhermes
dhermes reviewed Nov 14, 2016
View reviewed changes
monitoring/google/cloud/monitoring/query.py
new_metric_type = kwargs.get('metric_type')
if new_metric_type is not None:
self.metric_type = new_metric_type
kwargs.pop('metric_type', None)

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@supriyagarg supriyagarg force-pushed the monitoring_select_metrics branch 2 times, most recently from b7335ab to 84122a1 Compare November 15, 2016 01:00
dhermes
dhermes approved these changes Nov 15, 2016
View reviewed changes
Copy link
Contributor

@dhermes dhermes left a comment

Choose a reason for hiding this comment

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

LGTM other than one tiny nit

monitoring/google/cloud/monitoring/query.py
@@ -314,6 +316,20 @@ def select_metrics(self, *args, **kwargs):

metric.label.<label> = ends_with("<value>")

As a special case, ``"metric_type"`` is treated as a special
pseudo-label corresponding to the filter object ``metric.type``.
For example, ``metric_type=<value>`` generates::

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@supriyagarg
Allow using a metric type in the Query method select_metrics.
52f304f 
This is useful when a user wants to apply the exact same filters
(duration, aggregation, resource labels) for a different metric type.
@dhermes dhermes added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Nov 22, 2016
@lukesneeringer lukesneeringer added the priority: p2 Moderately-important priority. Fix may not be included in next release. label Apr 19, 2017
@lukesneeringer
Copy link
Contributor

Hi @supriyagarg,
It looks like we did you a disservice here, by approving your PR but failing to merge it. Sadly, now that nearly a year as past, the code has moved significantly.

We are still willing to accept this in principle, but can not make changes to your PR (since you have it configured not to allow this). Therefore, I am going to close this issue as stale for now. If you would like to make the necessary updates, though, we would be happy to consider doing this.

vchudnov-g pushed a commit that referenced this pull request Sep 20, 2023
@nnegrey @busunkim96
dialogflow: split up entity and entity type tests [(#2735)](GoogleClo…
6dd655d 
…udPlatform/python-docs-samples#2735)

* dialogflow: split up entity and entity type tests

* delete old file

* run black on new files
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dhermes dhermes dhermes approved these changes

Assignees
No one assigned
Labels
api: monitoring Issues related to the Cloud Monitoring API. cla: yes This human has signed the Contributor License Agreement. do not merge Indicates a pull request not ready for merge, due to either quality or timing. priority: p2 Moderately-important priority. Fix may not be included in next release.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@supriyagarg @lukesneeringer @dhermes @daspecster @googlebot