[foteinigk]

Introduce a new configuration setting: FETCH_TRANSLATION_TIMEOUT, which allows callers to define a maximum time (in seconds) to wait when fetching translations or locales from CDS.

Previously, if CDS continued returning 202 Accepted for an extended period (e.g., when a job is stuck in a stale state), the system would keep retrying indefinitely. This new setting provides an additional safety mechanism—similar to the behavior offered by the Transifex JavaScript SDK—by enforcing an upper bound on how long we wait for CDS to produce a final result.

The default value is 0, meaning no global timeout, preserving existing behavior unless explicitly configured.

---

Update: per-request timeouts

The fetch requests themselves had no timeout, so an unreachable CDS still blocked for the OS TCP timeout (2+ min). That stalled txc gunicorn workers at startup and failed a staging deploy.

• New TRANSIFEX_REQUEST_TIMEOUT / request_timeout, default (10, 60) (connect, read), applied to each fetch request.
• Request timeouts are logged as Timeout, and that language is marked not fetched.
• TRANSIFEX_FETCH_TIMEOUT is now one deadline for the whole fetch (languages + all translations). Languages not reached in time are skipped until the next sync.
• Fixed the TEMEOUT typo in the README.

Note: unlike TRANSIFEX_FETCH_TIMEOUT, the request timeout is on by default. That's a small behaviour change for SDK users: a request that used to wait indefinitely now fails after 10 s connecting or 60 s without data.

[foteinigk]

Non-blocking: with the default fetch_timeout=0, startup still waits forever if CDS keeps answering 202. There's no deadline, so this loop polls every RETRY_DELAY_SEC without end. NativeConfig.ready() → tx.fetch_translations() never returns, and a later sync gets stuck the same way. request_timeout only limits each single request. This isn't a regression: the old loop did the same, without even the sleep, and the JS SDK also defaults fetchTimeout to 0. But it's the failure this setting was added for, and only callers who set it are protected.

Two options:

1. Keep 0 and say so in the README (see the README comment). No behaviour change.
2. Default it in Django only, the path that blocks startup:

TRANSIFEX_FETCH_TIMEOUT = getattr(settings,
                                  'TRANSIFEX_FETCH_TIMEOUT',
                                  120)

I'd go with 1 and set it in txc when we bump the SDK. What do you think?

[deathbird]

Agreed, option 1: keeping 0 and documenting it (your README text, in 7b7db03). txc sets TRANSIFEX_FETCH_TIMEOUT explicitly.

[foteinigk]

Regression: fetch_all_langs=True with languages=None now raises. This line always iterates self.configured_language_codes; before, the fetch_all_langs path never read it. tx.init(None, token, fetch_all_langs=True) followed by tx.fetch_translations() raises TypeError: 'NoneType' object is not iterable. That's outside the try/except, so the call fails and the sync daemon logs an exception every cycle. Django always passes a list, so this hits direct SDK users.

Suggested change

	ordered = [code for code in self.configured_language_codes
	ordered = [code for code in (self.configured_language_codes or [])

Test:

    @patch('transifex.native.cds.requests.get')
    def test_fetch_all_langs_without_configured_languages(self, mock_get):
        mock_get.side_effect = [
            self._json_response(200, {'data': [{'code': 'el'}]}),
            self._json_response(200, {'data': {}}),
        ]
        cds_handler = CDSHandler(None, 'some_token',
                                 host='https://some.host',
                                 fetch_all_langs=True)

        assert cds_handler.fetch_translations() == {'el': (True, {})}

[deathbird]

Good catch, applied with your test in 7b7db03. This was a regression from the language-order change, thanks.

[foteinigk]

An invalid request_timeout makes every fetch fail, and the log blames the response. Unlike fetch_timeout, the value is stored as-is and only checked by requests on each call. Most bad values (0, True, '30', [10, 60], a 3-tuple) become a urllib3 ValueError, which lands in except (KeyError, ValueError) and is logged as Malformed response. With fetch_timeout set, a list or string fails earlier, in min() inside _bounded_timeout, and is logged as UnknownError. Either way the languages request fails, nothing is fetched, and the app serves source strings. 0 is the easy one to hit, because TRANSIFEX_FETCH_TIMEOUT = 0 means "no limit".

Suggestion: check the value once here, with the same check requests runs. Accept a list (from JSON/YAML config). On failure, log an error and use the default. None stays "no limit", which test_none_request_timeout_parts_are_capped_by_deadline expects.

Suggested change

	self.request_timeout = request_timeout
	self.request_timeout = self._parse_request_timeout(request_timeout)

from urllib3.util import Timeout as Urllib3Timeout  # urllib3 ships with requests

    @staticmethod
    def _parse_request_timeout(value):
        """Return `value` as a `requests` timeout (a number or a
        (connect, read) tuple, None for no limit), or DEFAULT_REQUEST_TIMEOUT
        if it is invalid, so a bad setting is reported once here instead of
        failing every request."""
        if isinstance(value, list):  # e.g. read from JSON or YAML
            value = tuple(value)
        try:
            connect, read = value if isinstance(value, tuple) else (value,) * 2
            # The check `requests` runs on every request, done once here
            Urllib3Timeout(connect=connect, read=read)
        except (TypeError, ValueError):
            logger.error('Invalid request_timeout `{}`, using the default '
                         '`{}`'.format(value, DEFAULT_REQUEST_TIMEOUT))
            return DEFAULT_REQUEST_TIMEOUT
        return value

Test (import DEFAULT_REQUEST_TIMEOUT from transifex.native.cds):

    @patch('transifex.native.cds.logger')
    def test_invalid_request_timeout_uses_default(self, patched_logger):
        for value, expected in (
            (None, None),
            (5, 5),
            ((3, None), (3, None)),
            ([3, 30], (3, 30)),
            (0, DEFAULT_REQUEST_TIMEOUT),
            ('30', DEFAULT_REQUEST_TIMEOUT),
            (True, DEFAULT_REQUEST_TIMEOUT),
            ((1, 2, 3), DEFAULT_REQUEST_TIMEOUT),
        ):
            cds_handler = CDSHandler(['el'], 'some_token',
                                     request_timeout=value)
            assert cds_handler.request_timeout == expected
        assert patched_logger.error.call_count == 4

[deathbird]

Good catch, applied with your test in 7b7db03.

[foteinigk]

Minor: a read timeout while the body downloads is logged as ConnectionError. With the default stream=False, requests.get() reads the body itself, and Response.content re-raises urllib3's ReadTimeoutError as requests.ConnectionError. So a slow CDS is logged like an unreachable one, although the PR description says request timeouts are logged as Timeout. Converting it here fixes fetch_languages and fetch_translations in one place (plus from urllib3.exceptions import ReadTimeoutError):

Suggested change

	response = requests.get(*args, **kwargs)
	try:
	response = requests.get(*args, **kwargs)
	except requests.ConnectionError as e:
	# requests reports a read timeout while downloading the body
	# as ConnectionError; raise it as the Timeout it is
	if e.args and isinstance(e.args[0], ReadTimeoutError):
	raise requests.ReadTimeout(*e.args)
	raise

Test:

    @patch('transifex.native.cds.logger')
    @patch('transifex.native.cds.requests.get')
    def test_read_timeout_in_body_is_logged_as_timeout(
        self, mock_get, patched_logger
    ):
        mock_get.side_effect = requests.ConnectionError(
            ReadTimeoutError(None, None, 'Read timed out.'))
        cds_handler = CDSHandler(['el'], 'some_token',
                                 host='https://some.host')

        assert cds_handler.fetch_translations('el') == {'el': (False, {})}
        patched_logger.error.assert_called_once_with(
            'Error retrieving translations from CDS: Timeout')

[deathbird]

Good catch, applied with your test in 7b7db03.

[foteinigk]

Docs: say that a slow response can exceed the fetch timeout, what 0 means at startup, and that the request timeout must be greater than 0. requests applies the read timeout to each socket read, not to the whole response. With 25 s left the cap is (10, 25), but a response that sends a byte every 20 s never trips it. That's unlikely with CDS, so documenting it is enough; streaming the body to enforce the deadline isn't worth the code.

Suggested change

	Approximate maximum time in seconds for one fetch of languages and
	translations from CDS, including retries. It can be exceeded somewhat, e.g. by
	DNS resolution or a host with several IP addresses. Languages are fetched in
	LANGUAGES order; those not reached in time are skipped until the next sync,
	or for the life of the process if TRANSIFEX_SYNC_INTERVAL is 0.
	0 (default) = no global timeout.
	- TRANSIFEX_REQUEST_TIMEOUT:
	Timeout in seconds for each request made when fetching from CDS, either a
	number or a `(connect, read)` tuple as accepted by `requests`.
	Defaults to `(10, 60)`.
	Approximate maximum time in seconds for one fetch of languages and
	translations from CDS, including retries. It can be exceeded, e.g. by DNS
	resolution, a host with several IP addresses, or a response that keeps
	sending data slowly (the read timeout applies between received bytes, not to
	the whole response). Languages are fetched in LANGUAGES order; those not
	reached in time are skipped until the next sync, or for the life of the
	process if TRANSIFEX_SYNC_INTERVAL is 0.
	0 (default) = no global timeout: if CDS keeps answering 202, the fetch, and
	so startup, waits until it stops. Set it when translations are fetched at
	startup, e.g. under gunicorn.
	- TRANSIFEX_REQUEST_TIMEOUT:
	Timeout in seconds for each request made when fetching from CDS, either a
	number greater than 0 or a `(connect, read)` tuple as accepted by `requests`.
	Invalid values are logged and the default is used. Defaults to `(10, 60)`.

[deathbird]

Good catch, applied with your test in 7b7db03.

[foteinigk]

changes LGTM !! 🚀