ggshield icon indicating copy to clipboard operation
ggshield copied to clipboard

chore: add HTTPAdapter to request session

Open sevbch opened this issue 1 month ago • 2 comments

Context

A customer reported this issue when scanning with ggshield, on a self-hosted instance (XXX= host url).

59089:605129 [E] ggshield.core.errors:209 status_code=None detail=HTTPSConnectionPool(host='XXX', port=443): Read timed out. (read
timeout=60)
2025-10-28 11:13:04 59089:605240 [W] urllib3.connectionpool:329 Connection pool is full, discarding connection: XXX. Connection pool size: 10
2025-10-28 11:13:23 59089:605238 [W] urllib3.connectionpool:329 Connection pool is full, discarding connection: XXX. Connection pool size: 10
Scanning... ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━╺━━ 93% 529 / 569
Error: Scanning failed: HTTPSConnectionPool(host='XXX', port=443): Read timed out. (read timeout=60)

What has been done

Adding a HTTPAdapter to the session configuration to better handle many parallel requests. The risk is that it could put additional pressure on the server. After reading this article, I left pool_connections to default value, as it shouldn't have much of an impact for our case, but increased pool_maxsize as we can see in the logs that this is the param that was blocking for the customer.

Another option was to increase the 60s timeout, but I'm afraid it would have too much impact. For instance, when a ggshield command fail, it could result in a longer waiting time for the caller to fetch the output.

I'm open to suggestions if you see another way of handling this.

Validation

Validated with the unit test.

PR check list

  • [x] As much as possible, the changes include tests (unit and/or functional)
  • [x] If the changes affect the end user (new feature, behavior change, bug fix) then the PR has a changelog entry (see doc/dev/getting-started.md). If the changes do not affect the end user, then the skip-changelog label has been added to the PR.

sevbch avatar Oct 29 '25 15:10 sevbch