add optional sample timeout to tox sample runner #17139
Conversation
kristapratico
requested review from
annatisch,
rakshith91 and
yunhaoling
as code owners
|
Do these samples run forever by design? Should we change the samples instead since we don't want users to run for instance, an infinite loop. What is the behavior of these samples when a user runs the sample? |
|
@rakshith91 , the receiving forever sample is by design. as far as I know customers usually use eventhub/servicebus as part of a micro server application which constantly pumps and handles messages from the service, and that's why we have the push receiving design in eventhub and the service bus iterator mode (also push-based receiving). so I prefer to keeping most of the receiving sample as they are for customers to start with. but you have brought a good point and I looked into samples which don't necessarily has to be receiving forever and could be moved into normal section:
@swathipil , let's update the tickets #16705 #16642 to address those two. |
rakshith91
left a comment
rakshith91
left a comment
There was a problem hiding this comment.
@yunhaoling thanks for the explanation.
LGTM
| if isinstance(timeout, tuple): | ||
| timeout, pass_if_timeout = timeout | ||
| else: | ||
| pass_if_timeout = True |
There was a problem hiding this comment.
@kristapratico , added the default here in order to not change the flow of the current code too much. Made both timeout and pass_if_timeout required params in run_check_call_with_timeout since it seems that there would be no reason both of those wouldn't be passed in. Does that seem reasonable?
There was a problem hiding this comment.
Sounds good to me!
There was a problem hiding this comment.
Ah just realized I can't approve my own PR. I'm ready to merge when you are.
No description provided.