Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

http_GET vs http_client.get #715

Open
Lucas-C opened this issue Jan 22, 2025 · 6 comments
Open

http_GET vs http_client.get #715

Lucas-C opened this issue Jan 22, 2025 · 6 comments

Comments

@Lucas-C
Copy link

Lucas-C commented Jan 22, 2025

Why are there two methods to perform HTTP calls?

I wrote some code to test both: https://github.com/sncf-connect-tech/motis/blob/test-http/exe/main.cc#L143

The problem with http_GET() is that it does not support proxies.
It's used for example in https://github.com/motis-project/motis/blob/master/src/rt_update.cc#L76.
We would like to use a proxy in that case, so using http_client would be handier.

http_GET does not seem used anywhere else.

Would you be OK to replace http_GET by http_client.get() in src/rt_update.cc,
and use the proxy in the config file if it's set?


Side notes:

Once compiled, the modified motis command in https://github.com/sncf-connect-tech/motis/tree/test-http can be used like this:

build/motis test-http_client
build/motis test-http_GET

The first command seems to "hang" in my environment.

Also, when launching a local Python HTTP server with python -m http.server (tested with Python 3.10 & 3.12),
if I target http://localhost:8000 in test-http_GET instead of https://httpbin.org/get, I get this error:

Starting coroutine...
Starting HTTP GET request...
Coroutine failure: bad version [beast.http:14 at /opt/motis/deps/boost/boost/beast/http/detail/basic_parser.ipp:357:9 in function 'static void boost::beast::http::detail::basic_parser_base::parse_version(const char*&, const char*, int&, boost::beast::error_code&)']
@Lucas-C
Copy link
Author

Lucas-C commented Jan 22, 2025

Bonus: maybe http_client should be added to https://github.com/motis-project/utl?

@felixguendling
Copy link
Member

Would you be OK to replace http_GET by http_client.get() in src/rt_update.cc,
and use the proxy in the config file if it's set?

I think this makes sense.
Any objections @pablohoch?

Bonus: maybe http_client should be added to https://github.com/motis-project/utl?

It could replace our own HTTP client implementation in motis-project/net.

@felixguendling
Copy link
Member

felixguendling commented Jan 22, 2025

The first command seems to "hang" in my environment.

The http_client is supposed to keep the TCP connection open.
Maybe the channels keep the I/O service running.


But: I experienced problems in the past with the number of open connections exceeding 1024. I was not able to find the cause. So for now I just used this SystemD config:

[Service]
LimitNOFILE=65536
...

(you can achieve the same with ulimit)

@pablohoch
Copy link
Member

Would you be OK to replace http_GET by http_client.get() in src/rt_update.cc,
and use the proxy in the config file if it's set?

I think this makes sense. Any objections @pablohoch?

Agreed.

Also, when launching a local Python HTTP server with python -m http.server (tested with Python 3.10 & 3.12), if I target http://localhost:8000 in test-http_GET instead of https://httpbin.org/get, I get this error:

Starting coroutine...
Starting HTTP GET request...
Coroutine failure: bad version [beast.http:14 at /opt/motis/deps/boost/boost/beast/http/detail/basic_parser.ipp:357:9 in function 'static void boost::beast::http::detail::basic_parser_base::parse_version(const char*&, const char*, int&, boost::beast::error_code&)']

I've encountered this error after sending multiple requests using HTTP pipelining (with http_client) to some hosts, which led me to believe that it could be a server issue (was using SSL for that, so not that easy to debug). If the error can be reproduced using a single HTTP request, it's probably a client bug.

@Lucas-C
Copy link
Author

Lucas-C commented Jan 22, 2025

It could replace our own HTTP client implementation in motis-project/net.

Alright.
This HTTP client in motis-project/net is currently unused?

@felixguendling
Copy link
Member

MOTIS currently only uses web_server and query_router from the net library.

@Lucas-C Lucas-C changed the title http_GET vs htt_client.get http_GET vs http_client.get Jan 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants