uhd-urz / elAPI

An extensible API client for eLabFTW
GNU Affero General Public License v3.0
5 stars 0 forks source link

Disable HTTP/2 support - [merged] #102

Closed alexander-haller closed 1 month ago

alexander-haller commented 1 month ago

In GitLab by @mhxion on Jun 13, 2024, 23:34

Merges revert-http2 -> dev

Fixes #46.

alexander-haller commented 1 month ago

In GitLab by @mhxion on Jun 13, 2024, 23:34

requested review from @alexander-haller

alexander-haller commented 1 month ago

In GitLab by @mhxion on Jun 14, 2024, 24:12

We introduce a new field enable_http2 for elapi.yml. We keep the default value False. This would mainly be for the public; if someone wants to use HTTP/2.

Implementing this didn't take too much time. It is ready as well if you think it's worth the feature. I should mention, the error reported in #46 doesn't occur all the time, but for sake of the criticality of bill-teams, it'd be best keep HTTP/2 disabled by default until a fix is figured out by HTTPX.

alexander-haller commented 1 month ago

In GitLab by @mhxion on Jun 14, 2024, 24:23

Did some very simple tests, and HTTP/2 is only 7% faster for bill-teams teams-info (consisted on production and dev server, dev-002 just fails).

alexander-haller commented 1 month ago

If HTTP/2 does not introduce too much maintenance/additional code paths that need to be carried in the future we can keep it as feature for others. Internally I would keep enable_http2=False it for the time being if this is not stable.

alexander-haller commented 1 month ago

In GitLab by @project_994_bot_1c5bd6ac1fdbd740fbc4ed080ecff58d on Jun 14, 2024, 14:11

added 1 commit

Compare with previous version

alexander-haller commented 1 month ago

approved this merge request

alexander-haller commented 1 month ago

In GitLab by @mhxion on Jun 14, 2024, 18:10

I couldn't reproduce the RemoteProtocolError we saw happen with elapi bill-teams teams-info at home. I am going to leave everything as is, and merge this branch for now (also with default enable_http2 value as False, it literally doesn't change anything for bill-teams, so the culprit for RemoteProtocolError shouldn't be HTTP/2 of this PR).

alexander-haller commented 1 month ago

In GitLab by @mhxion on Jun 14, 2024, 19:15

added 3 commits

Compare with previous version