Python API: move the really_get_paths function into the API
#41
Conversation
While this is a terrible idea with the current implementation, I believe that having a "block until you get some paths" function in the API makes sense. Thus, see the doc string of that function.
There was a problem hiding this comment.
Reviewed 3 of 3 files at r1, 1 of 2 files at r2.
Reviewable status: 3 of 4 files reviewed, all discussions resolved (waiting on @juagargi)
| @@ -8,7 +8,7 @@ def main(): | |||
| print('Local Address is %s' % local_address()) | |||
|
|
|||
| destination = '17-ffaa:1:a,[127.0.0.1]:12345' | |||
| p = paths(destination) | |||
| p = get_paths(destination, loop_till_have_paths=False) | |||
There was a problem hiding this comment.
Why did we pass False here?
| @@ -17,7 +17,7 @@ | |||
| """ | |||
|
|
|||
| __all__ = ['SCIONException', 'HostInfo', 'FwdPathMeta', 'Interface', 'Path', 'connect', | |||
| 'set_log_level', 'init', 'local_address', 'paths', 'listen'] | |||
| 'set_log_level', 'init', 'local_address', 'get_paths', 'listen'] | |||
There was a problem hiding this comment.
Sorry, forgot about that, thanks!
| except sci.SCIONException: | ||
| time.sleep(0.1) | ||
| return _call_paths(destination) | ||
| except SCIONException as e: |
There was a problem hiding this comment.
why did we delete the sleep here?
go/pyscion.py
Outdated
| @@ -270,10 +270,9 @@ def get_paths(destination, loop_till_have_paths=True): | |||
| Ideally, this function would be able to distinguish between "no paths in | |||
| cache" and "unreachable". That is a TODO. | |||
| """ | |||
| if not loop_till_have_paths: return paths(destination) | |||
There was a problem hiding this comment.
why remove this and add the special case for exception handling below, if we can just keep this case here and save ourselves the special case below?
There was a problem hiding this comment.
I've answered comments in reviewable
Reviewed 1 of 2 files at r2.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @AnotherKamila)
go/example.py, line 11 at r2 (raw file):
Previously, AnotherKamila (Kamila Součková) wrote…
Why did we pass False here?
I want the parameter to be very visible in the example, and it is false because typically when run for the first time, it will fail (misconfigured AS, etc). Otherwise it gets stuck.
go/pyscion.py, line 273 at r1 (raw file):
cache" and "unreachable". That is a TODO. """ if not loop_till_have_paths: return paths(destination)
so you @AnotherKamila were asking in a different comment why I removed the special handling of the case when we don't loop until we have paths. We have now a do ... while sort of loop, with only one call to _call_paths, while before there was a special branch dedicated to handle the case for loop. The exception handling happens only if we loop, and whenever we change _call_paths to return an empty pathset instead of an exception, this is my intention (one control branching):
while True:
p = _call_paths(destination)
if not loop_till_have_paths or len(p) > 0:
return pJust explaining myself. Not that this is hugely important, if you hate it, we can revert it back to your original.
go/pyscion.py, line 20 at r2 (raw file):
Previously, AnotherKamila (Kamila Součková) wrote…
Sorry, forgot about that, thanks!
no problem. BTW your comment is set to "blocking"
go/pyscion.py, line 276 at r2 (raw file):
Previously, AnotherKamila (Kamila Součková) wrote…
why did we delete the sleep here?
the question is why did we sleep? If a sleep is necessary, we would have to know for how long. The call to _call_paths is supposed to have its own internal timeout already.
While this is a terrible idea with the current implementation, I believe that
having a "block until you get some paths" function in the API makes sense. Thus, I propose this as a temporary workaround -- see the doc string of that function.
(We all know how temporary workarounds work :-) )
This change is