RFC: Blocking queries index param #89
Conversation
Simply checking `index` for True results in an empty dictionary also being considered as not being used.
Codecov Report
@@ Coverage Diff @@
## master #89 +/- ##
==========================================
- Coverage 91.23% 91.09% -0.15%
==========================================
Files 26 26
Lines 1038 1044 +6
==========================================
+ Hits 947 951 +4
- Misses 91 93 +2
Continue to review full report at Codecov.
|
| @@ -10,12 +10,13 @@ class Requester(object): | |||
|
|
|||
| ENDPOINT = "" | |||
|
|
|||
| def __init__(self, address=None, uri='http://127.0.0.1', port=4646, namespace=None, token=None, timeout=5, version='v1', verify=False, cert=(), region=None, **kwargs): | |||
| def __init__(self, address=None, uri='http://127.0.0.1', port=4646, namespace=None, token=None, timeout=5, blocking_timeout=360, version='v1', verify=False, cert=(), region=None, **kwargs): | |||
There was a problem hiding this comment.
Would we want to add this to nomad/__init__.py to pass this when creating all the endpoint objects so the blocking_timeout could be adjusted on the constructor?
There was a problem hiding this comment.
Yes we would! That was purely an oversight on my behalf after setting a default which was usable for my purposes.
There was a problem hiding this comment.
This looks great @damoxc! Just the one question about customizing the blockout time and if you would have any recommendations on the endpoints that do support blocking to test that on an integration level? We could totally do unit as well.
|
Regarding the timeouts, the blocking endpoints also support a
So I think from our side we would want to enforce the Testing wise I imagine the only thing possible to test would be that we correctly calculate the blocking_timeout, from my understanding when a blocking request returns it may or may not have changes making it tricky to assert something. If it's possible to start a blocking call then alter the object in question via another client instance we could then check our blocking client also returns? |
|
Hey @damoxc, so I was trying around the wait querystring parameter against the Nomad API. It doesn't look like it the input is enforced. I can give it a number beyond its limit of 10m, but will still return at the 10 minute mark. I'm not sure if we need to enforce that detail other than just providing the wait parameter for the constructor. Also sorry for the delayed responses i'll try to be better about response time. |
I've tried to implement this in a backwards compatible way, without adjusting the return value from API calls, so using a dictionary to essentially pass the index parameter by reference rather than value making it possible to update.
Interested in your thoughts on it.