search

BlueBrain / nexus-forge

Building and Using Knowledge Graphs made easy
https://nexus-forge.readthedocs.io
GNU Lesser General Public License v3.0
38 stars 19 forks source link

added timeouts to every requests. call #368

Closed ssssarah closed 8 months ago

ssssarah commented 9 months ago

There have been some issues recently in order to access some things hosted on github.io (ex: metadata context) (they should be fixed by the time of this review), and it's brought out one issue in forge: there were no timeouts when making requests, and some forge calls could hang for close to 10 minutes before getting an error that the server could not be reached.

This MR sets a timeout of 60 seconds for every request call in the codebase (constants are set in the different parts of forge that make requests, so that they may be changed depending on the use case)

crisely09 commented 8 months ago

Is there a reason not to have a global TIMEOUT variable, that could be overwritten for each case? should it be accesible to be changed by the user?

ssssarah commented 8 months ago

Is there a reason not to have a global TIMEOUT variable, that could be overwritten for each case? should it be accesible to be changed by the user?

I would think this offers flexibility and that different implementations may go for their own timeout, ex: fetching a file could have a different timeout than having a response with json in the body.

The flexibility can still be kept by having each location have their own local variable that gets (for now) as its value the default one set somewhere else. Should one want to change its value, only the file/class local variable would need to be overridden, the request implementation wouldn't need to be changed and the global default variable wouldn't need to be changed either.

codecov-commenter commented 8 months ago

Codecov Report

Attention: 5 lines in your changes are missing coverage. Please review.

Comparison is base (fc39f07) 74.51% compared to head (79bd75e) 74.55%.

Files Patch % Lines
kgforge/specializations/stores/bluebrain_nexus.py 40.00% 3 Missing :warning:
kgforge/core/commons/files.py 66.66% 1 Missing :warning:
..._linking/service/entity_linking_elastic_service.py 66.66% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #368 +/- ## ========================================== + Coverage 74.51% 74.55% +0.04% ========================================== Files 100 101 +1 Lines 6332 6344 +12 ========================================== + Hits 4718 4730 +12 Misses 1614 1614 ``` | [Flag](https://app.codecov.io/gh/BlueBrain/nexus-forge/pull/368/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=BlueBrain) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/BlueBrain/nexus-forge/pull/368/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=BlueBrain) | `74.55% <75.00%> (+0.04%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=BlueBrain#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.