fix(client): add timeout override to create_bitstream

[jr-rk]

Summary

• create_bitstream gained a timeout param (default None -> falls back to self.timeout), matching the pattern already used by api_post.
• CSRF-retry recursion now propagates the caller-supplied timeout instead of dropping it on retry.

Why

Large bitstream uploads (600+ MB) were aborting mid-write with ConnectionError: The write operation timed out against the client's flat 60s default. Consumer (DSpace-ISstag-integration) now scales timeout per file size when calling create_bitstream.

Test plan

• Existing client tests pass
• Manual: large-file bitstream upload from downstream importer no longer times out at 60s

🤖 Generated with Claude Code

[Copilot]

🟡 Changes recommended

Retry handling can raise a TypeError on non-JSON 401/403 responses and the new timeout override behavior lacks targeted test coverage.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR extends the REST client’s create_bitstream API to accept a per-call timeout override (falling back to self.timeout) and ensures CSRF/auth retry logic preserves the caller’s timeout, addressing large-upload write timeouts.

Changes:

• Add timeout parameter to create_bitstream() and pass it through to requests.Session.send(...).
• Propagate the caller-provided timeout through the CSRF/auth retry recursion path.

File summaries

File	Description
dspace_rest_client/client.py	Adds an optional per-call timeout to bitstream uploads and ensures retries keep the same timeout.

Review details

Suppressed comments (2)

dspace_rest_client/client.py:881

• The retry call uses positional arguments, which is now easier to get wrong with the added timeout parameter; using keywords makes the retry logic clearer and more robust to future signature changes.

            return self.create_bitstream(bundle, name, path, mime, metadata, True, timeout)

dspace_rest_client/client.py:869

• The new per-call timeout override is behaviorally important (prevents mid-upload timeouts) but isn’t covered by tests. There are existing create_bitstream tests; add one that passes timeout=... and asserts session.send(..., timeout=...) received it (e.g., by wrapping/mocking c.session.send).

            r = self.session.send(prepared_req, proxies=self.proxies,
                                  timeout=timeout if timeout is not None else self.timeout)

• Files reviewed: 1/1 changed files
• Comments generated: 2
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟡 Changes recommended

There are a couple of small but concrete robustness/maintainability issues in the updated CSRF retry block (type-safety of JSON handling and brittle positional recursion) that should be addressed before approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

dspace_rest_client/client.py:885

• The CSRF-detection condition can still raise errors if the server returns valid JSON that isn’t a dict (e.g., a JSON string): 'message' in (r_json or {}) may evaluate true, but r_json['message'] will then fail. Guard on isinstance(r_json, dict) before indexing.

            r_json = parse_json(r)
            if 'message' in (r_json or {}) and 'CSRF token' in r_json['message']:
                _logger.debug("Retrying request with updated CSRF token")

• Files reviewed: 2/2 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

The new test asserts requests_mock timeout via m.request_history[i].timeout, which is likely not a stable/available attribute and may fail CI.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

tests/test_client_write.py:213

• requests_mock request history objects don’t reliably expose a .timeout attribute, so asserting m.request_history[i].timeout is likely to raise AttributeError (and even if it exists in some versions, it’s not a stable part of the public API). Capture the timeout in a version-agnostic way by wrapping c.session.send and asserting the timeout kwarg passed on each call.

• Files reviewed: 2/2 changed files
• Comments generated: 0 new
• Review effort level: Lite