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.

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

Fix Timeseries Query in Monitoring System Test #2460

Closed
wants to merge 2 commits into from

Conversation

waprin
Copy link
Contributor

@waprin waprin commented Sep 28, 2016

See #2459 .

The old method was querying with the wrong interval, but the test still passed because new query objects were being unnecessarily created, and the end time would eventually roll over so the interval was correct. Besides being wrong, this also led to longer back off then necessary since results might have been available before the minute rollover.

This should be tweaked again if we decided to change Query object to not replace seconds. But either way we definitely only want to create the Query object once and not retry it, and in the meantime this patch should speed up the system test.

cc @supriyagarg @rimey

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Sep 28, 2016
max_tries=MAX_RETRIES)(client.query)
return RetryErrors(BadRequest, max_tries=MAX_RETRIES)(retry_result)
endtime = datetime.datetime.utcnow()
endtime += datetime.timedelta(minutes=1)

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

Bill Prin added 2 commits September 28, 2016 13:33
The old method was querying with the wrong
interval, but the test still passed because
new query objects were being unnecessarily
created, and the end time would eventually roll
over so the interval was correct. Besides being
wrong, this also led to longer back off then
necessary since results might have been available
before the rollover.
return len(list(result)) > 0
# need to wrap built-in function for decorators to work
def list_timeseries(query):
return list(query)

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

@tseaver tseaver added the api: monitoring Issues related to the Cloud Monitoring API. label Sep 29, 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

This issue may no longer be relevant due to its age. Feel free to re-open.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: monitoring Issues related to the Cloud Monitoring API. cla: yes This human has signed the Contributor License Agreement. priority: p2 Moderately-important priority. Fix may not be included in next release.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants