[asset] Add sample code for two new RPCs. #4080
Conversation
googlebot
added
the
cla: yes
yuyifan-google
changed the title
|
Do you know why the kokoro tests are not running? |
busunkim96
added
the
kokoro:force-run
kokoro-team
removed
the
kokoro:force-run
|
@yuyifan-google They only run automatically for the members of the |
busunkim96
added
the
kokoro:force-run
kokoro-team
removed
the
kokoro:force-run
| query = "name:{}".format(DATASET) | ||
|
|
||
| @backoff.on_exception( | ||
| backoff.expo, (AssertionError, InvalidArgument), max_time=30 |
There was a problem hiding this comment.
Why are we added a backoff for this sample? Can we add a comment with context on why this is?
There was a problem hiding this comment.
It takes some time for the created dataset to be searchable. I realized sleep() may be more intuitive in this case than using backoff to retry the search.
There was a problem hiding this comment.
Sorry, but why? I recommend backoff over sleep.
There was a problem hiding this comment.
Ok I'll switch back to backoff. I don't code python a lot so may I ask why backoff is preferred?
There was a problem hiding this comment.
Using backoff is tmatsuo's personal preference, and it's not one I agree with here. I believe backoff.on_exception has max_tries default to None, which means this will never fail and instead will continue to retry until the whole build fails out.
There was a problem hiding this comment.
I have max_time set to 30. It take no more than a few seconds to propagate, so maybe reduce it to 10s?
There was a problem hiding this comment.
I'm fine with 30 - the difference seems negligible.
| @backoff.on_exception( | ||
| backoff.expo, (AssertionError), max_time=30 | ||
| ) |
There was a problem hiding this comment.
Is there a way to tell if this dataset is searchable as part of the fixture? I would much prefer a check at the fixture level to determine if the resource is searchable before yielding it. If that can only be accomplished by searching it, it's fine to leave this as is (but please make sure to include either max_time or max_tries so it doesn't timeout forever).
There was a problem hiding this comment.
I don't think it's possible unless we do sleep() for a few seconds.
kurtisvg
added
the
kokoro:force-run
kokoro-team
removed
the
kokoro:force-run
No description provided.