fabric8-analytics / fabric8-analytics-server

fabric8-analytics API server
Apache License 2.0
16 stars 56 forks source link

Please specify timeout for the Prometheus service HTTP POST call #550

Open tisnik opened 5 years ago

tisnik commented 5 years ago

https://github.com/fabric8-analytics/fabric8-analytics-server/blob/a9330f79207c795366a0f04001b82219d7d73f5f/bayesian/api_v1.py#L272

Most requests to external servers should have a timeout attached, in case the server is not responding in a timely manner. By default, requests do not time out unless a timeout value is set explicitly. Without a timeout, your code may hang for minutes or more.

miteshvp commented 5 years ago

It's an async future requests call. API server does not wait for response. Closing this.

tisnik commented 5 years ago

@miteshvp is not this session used to call workers asynchronously? I'd suggest to use second FutureSession instance for calling Prometheus

miteshvp commented 5 years ago

Actually, its threaded. https://github.com/fabric8-analytics/fabric8-analytics-server/blob/master/bayesian/api_v1.py#L81-L82 so we don't need any other session.

tisnik commented 5 years ago

Yup, I meant that we might need to increase the number of workers for the futuresession?

miteshvp commented 5 years ago

@tisnik - I guess 100 is a good enough number for our requests. These sessions are not used to invoke workers at the moment. Workers are invoked via another async flow https://github.com/fabric8-analytics/fabric8-analytics-server/blob/master/bayesian/api_v1.py#L278 I guess we are good here.

tisnik commented 5 years ago

Yup, and what timeout value to choose? In this case it might be let's say from one to five seconds at most. WDYT?

miteshvp commented 5 years ago

@tisnik - this is like a nano-sec call. Timeout doesn't make sense here

tisnik commented 5 years ago

@miteshvp yes - in case everything is working as expected. But in reality we'll see network issues that would need to be handled by our service. We can discuss in in a chat rather than there ;)