X Tutup
Skip to content

Add More Retries to 400 on Query#2412

Merged
dhermes merged 1 commit intogoogleapis:masterfrom
waprin:fix_400
Sep 24, 2016
Merged

Add More Retries to 400 on Query#2412
dhermes merged 1 commit intogoogleapis:masterfrom
waprin:fix_400

Conversation

@waprin
Copy link
Contributor

@waprin waprin commented Sep 23, 2016 •
edited by dhermes
Loading

Fixes #2402.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Sep 23, 2016
@waprin
Copy link
Contributor Author

waprin commented Sep 23, 2016

@dhermes Can't do much about all the retrying but I think this will fix the test.

dhermes
dhermes reviewed Sep 23, 2016
View reviewed changes
system_tests/monitoring.py
retry_404_500 = RetryErrors((NotFound, InternalServerError))
retry_400_500 = RetryErrors((BadRequest, InternalServerError))
retry_500 = RetryErrors(InternalServerError)
retry_503 = RetryErrors(ServiceUnavailable)

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@dhermes
Copy link
Contributor

dhermes commented Sep 23, 2016

Thanks for the quick turnaround @waprin!

@dhermes
Copy link
Contributor

dhermes commented Sep 23, 2016

Can't do much about all the retrying but I think this will fix the test.

You can tweak the retry defaults like the initial delay and the number of retries.

@dhermes
Copy link
Contributor

dhermes commented Sep 23, 2016

@waprin This doesn't fix the issue reported in #2402.

I just checked out the stacktrace and it happens when the iterator is cast to a list: https://github.com/GoogleCloudPlatform/google-cloud-python/blob/af4f346e238e1a45ff5754335b9f8a4d98708e49/system_tests/monitoring.py#L212

@waprin waprin changed the title Add 400 Retry to Monitoring Write Point System Test Add More Retries to 400 on Query Sep 23, 2016
@waprin
Copy link
Contributor Author

waprin commented Sep 23, 2016

@dhermes does it look right to you now?

dhermes
dhermes reviewed Sep 23, 2016
View reviewed changes
system_tests/monitoring.py Outdated
retry_result = RetryResult(_has_timeseries, max_tries=7)(
client.query)
return RetryErrors(BadRequest)(retry_result)
return RetryErrors(BadRequest, max_tries=7)(retry_result)

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@dhermes
Copy link
Contributor

dhermes commented Sep 23, 2016

LGTM pending Travis.

Can you

  1. Ping me once Travis goes green (I'll merge for you)
  2. File an issue to make the blunt instrument sharper (i.e. blunt thing == these error retries)

@waprin
Copy link
Contributor Author

waprin commented Sep 24, 2016

@dhermes travis green

@dhermes dhermes merged commit 5edcaaf into googleapis:master Sep 24, 2016
@tseaver tseaver added api: monitoring Issues related to the Cloud Monitoring API. flaky labels Sep 24, 2016
@tseaver tseaver mentioned this pull request Sep 24, 2016
Closed
@dhermes dhermes mentioned this pull request Sep 29, 2016
Closed
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 left review comments

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.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@waprin @dhermes @tseaver @googlebot
X Tutup