Skip to content

Comments

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

Closed
supriyagarg wants to merge 2 commits intogoogleapis:masterfrom
supriyagarg:monitoring_select_metrics
Closed

Allow using a metric type in the Query method select_metrics.#2735
supriyagarg wants to merge 2 commits intogoogleapis:masterfrom
supriyagarg:monitoring_select_metrics

Conversation

@supriyagarg
Copy link
Contributor

@supriyagarg supriyagarg commented Nov 14, 2016

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

supriyagarg commented Nov 14, 2016

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

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.
@supriyagarg supriyagarg force-pushed the monitoring_select_metrics branch from 84122a1 to 52f304f Compare November 22, 2016 00:12
@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

lukesneeringer commented Aug 7, 2017

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

1 more reviewer

@dhermes dhermes dhermes approved these changes

Reviewers whose approvals may not affect merge requirements

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