gacou54/pyorthanc

HTTPX Timeout setting not synced w/ Orthanc setting

Closed this issue · 5 comments

I'm not sure how many calls this might apply to, but when calling a C-MOVE from a remote modality, the timeout setting in the data dict appears to be just for Orthanc. HTTPX has it's own timeout default of 5s, so if the requested timeout is larger than that and the request does not resolve in 5s, an HTTPX timeout occurs. Using the C-MOVE in the readme as a start:

from pyorthanc import RemoteModality, Orthanc

orthanc = Orthanc('http://localhost:8042', 'username', 'password')

modality = RemoteModality(orthanc, 'modality')

# Query (C-Find) on modality
data = {'Level': 'Study', 'Query': {'PatientID': '*'}}
query_response = modality.query(data=data)

answer = modality.get_query_answers()[query_response['ID']]
print(answer)

# Retrieve (C-Move) results of query on a target modality (AET)
modality.move(query_response['ID'], {'Timeout': 300})   # <-here's the problem

I'm working on a VPN w/ a slow connection, so the move command takes a while.
Without modification, this will always raise an HTTPX timeout, regardless of the timeout setting (since HTTPX is not being passed this setting). It seems like a relatively easy fix; something like:

timeout = cmove_data.get("Timeout", 5)
dict(self.client.post_modalities_id_query(self.modality, json=data, timeout=timeout))

here:

return dict(self.client.post_modalities_id_query(self.modality, json=data))

You'd need to adjust post_modalities_id_query and self._post as well to handle the parameter, but this would at least make HTTPX use the same timeout as requested for Orthanc.

When I adjusted this manually by removing the timeout (by adding timeout=None) the C-MOVE was successful given enough time.

Hi @jrkerns ,

That's an excellent point, we should be able to change the timeout of a RemoteModality. I think it would be better if the Orthanc class had a timeout parameter, so every call made with the client instance would use the timeout. I actually plan to add all the httpx.Client parameters in the Orthanc.__init__() for the next release (hopefully next week). Then, this would work:

orthanc = Orthanc('url', 'username', 'password', timeout=300)
modality = RemoteModality(orthanc, 'mymodality')

# Now all orthanc and modality calls should have the 300 s timeout 

For now, this should do the trick since Orthanc inherited from httpx.Client:

orthanc = Orthanc('url', 'username', 'password')
orthanc.timeout = 300
modality = RemoteModality(orthanc, 'mymodality')

Thanks, that's definitely easy. I ended up overriding .post :

class MyOrthanc(Orthanc):

   def post(*args, **kwargs):
        return super().post(*args, **kwargs, timeout=300)

but I like your solution more.

I have the same problem, need to set the httpx timout because the default is much too short. Hope this will be released soon. Thanks a lot!

Hi @Kiechlus,

PR #26 is ongoing, I hope to release the new pyorthanc version this week. In the meantime, this should work

orthanc = Orthanc('url', 'username', 'password')
orthanc.timeout = 300  # Set the timeout

# now use orthanc as you would ordinary
orthanc.get_patients()
...

Thank you for your interest to make pyorthanc better!

timeout added in #26