Remove timeouts from TP requests

[pnacht]

This PR removes the timeouts used in both trusted-publishing requests: the GET requesting an OIDC token, and the POST requesting the actual API token.

I have noticed that when a single identity is registered as the trusted publisher for many packages (200+), twine can occasionally* timeout. I have therefore had to start performing the token exchange myself again, as we had to do before v6.10.

I suspect this is because the API token is a macaroon, and the server must therefore identify all the packages that have registered that same identity as a "trusted publisher", and then store them in the macaroon. I assume this is computationally expensive, so, with many packages (and/or if the server is busy with many other requests), the current hardcoded timeouts become insufficient.

Now, the better solution here would be to allow users to pass a custom timeout: twine upload --timeout 30. However, I've noticed that none of the other requests performed by twine have timeouts:

twine/twine/repository.py

Lines 102 to 107 in 9175334

	resp = self.session.post(
	self.url,
	data=encoder,
	allow_redirects=False,
	headers={"Content-Type": encoder.content_type},
	)

twine/twine/repository.py

Lines 151 to 156 in 9175334

	resp = self.session.post(
	self.url,
	data=monitor,
	allow_redirects=False,
	headers={"Content-Type": monitor.content_type},
	)

twine/twine/repository.py

Line 215 in 9175334

response = self.session.get(url, headers=headers)

I have therefore chosen to simply remove the timeouts to be consistent with the rest of the codebase. This is largely because I'm not familiar enough with twine's codebase to make the larger changes necessary to make a potential --timeout argument apply everywhere (as users would expect).

If preferred and if someone can point me in the right direction(s), I'll be happy to set a user-defined global timeout everywhere instead.

---

* I haven't kept track of how often it happens... it's just common enough to be annoying... maybe 10% of the time?

[sigmavirus24]

I'd argue that if Warehouse can't satisfy a request within 5 seconds, that's alarming as well. Have you opened an issue against them as well?

[woodruffw]

Huh, I wouldn't expect that inclusion check itself to be that expensive on Warehouse, so that is indeed concerning. The macaroon serialization itself should also be pretty fast, but who knows.

+1 to filing this upstream as well -- I imagine the PyPI admins already have some inkling of this if it's related to the use case I suspect (the same usecase that caused an issue on PyPI last month with macaroons that were too big for HTTP headers), but it would be good to have it tracked there as a performance issue specifically.

(With that said, I'm not opposed to dropping this timeout or perhaps doubling to 10s to see if that's "good" enough. But I think we should definitely get a better understanding of the cause here first 🙂)

[sigmavirus24]

I'm also not confident that the lack of timeouts elsewhere is sufficiently motivating to remove these timeouts. If rather have timeouts everywhere. Also a single timeout value for all won't be correct

[pnacht]

Created pypi/warehouse#18514.