-
Notifications
You must be signed in to change notification settings - Fork 6.2k
fix(testing): add configurable request timeout to cucumber API POST step #5378
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds a configurable request timeout to the Cucumber/Behave API POST step to prevent test runs from hanging indefinitely when the backend takes excessively long to respond. The change addresses a specific issue where Docker-Compose test runs could hang on certain scenarios like split-pdf-by-sections, causing CI pipeline failures.
- Added configurable
request_timeoutparameter to the POST request step with a 60-second default - The timeout can be customized via
context.request_timeoutor defaults to 60 seconds if not set - Uses
getattr()for safe attribute access with a default fallback
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| + " Input:PDF Output:ZIP-PDF Type:SISO") | ||
| public ResponseEntity<StreamingResponseBody> splitPdf( | ||
| @ModelAttribute SplitPdfBySectionsRequest request) throws Exception { | ||
| public ResponseEntity<?> splitPdf(@ModelAttribute SplitPdfBySectionsRequest request) |
Copilot
AI
Jan 1, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The return type has been changed from ResponseEntity<StreamingResponseBody> to ResponseEntity<?>. While this is more flexible, using a wildcard ? is less specific and doesn't provide type safety. Consider using a concrete return type like ResponseEntity<byte[]> or ResponseEntity<Resource> to maintain type safety and make the API contract clearer.
| public ResponseEntity<?> splitPdf(@ModelAttribute SplitPdfBySectionsRequest request) | |
| public ResponseEntity<byte[]> splitPdf(@ModelAttribute SplitPdfBySectionsRequest request) |
| try (PDDocument mergedDoc = | ||
| pdfDocumentFactory.createNewDocumentBasedOnOldDocument( | ||
| sourceDocument); |
Copilot
AI
Jan 1, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The document creation method has changed from pdfDocumentFactory.createNewDocument() to pdfDocumentFactory.createNewDocumentBasedOnOldDocument(sourceDocument). This change could affect the behavior of the merged document by potentially inheriting metadata, document catalog settings, or other properties from the source document. Please verify that this change is intentional and that it doesn't introduce unwanted side effects. If the old behavior of creating a completely fresh document is required, consider reverting to createNewDocument().
| try (PDDocument mergedDoc = | |
| pdfDocumentFactory.createNewDocumentBasedOnOldDocument( | |
| sourceDocument); | |
| try (PDDocument mergedDoc = pdfDocumentFactory.createNewDocument(); |
| from PIL import Image, ImageDraw | ||
|
|
||
| API_HEADERS = {"X-API-KEY": "123456789"} | ||
| TIMEOUT = 80 # seconds |
Copilot
AI
Jan 1, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The timeout value is set to 80 seconds, but the PR description states "a default of 60 seconds". This inconsistency between the code and documentation should be resolved. Either update the constant to 60 seconds to match the PR description, or update the PR description to reflect the actual 80-second timeout.
| TIMEOUT = 80 # seconds | |
| TIMEOUT = 60 # seconds |
Replaces getPagesToSplit with getCustomPagesToSplit and introduces shouldSplitPage to centralize split mode logic. This refactor improves code readability and maintainability by separating custom page selection from split mode handling.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
🚀 V2 Auto-Deployment Complete!Your V2 PR with the new frontend/backend split architecture has been deployed! 🔗 Direct Test URL (non-SSL) http://185.252.234.121:5378 🔐 Secure HTTPS URL: https://5378.ssl.stirlingpdf.cloud This deployment will be automatically cleaned up when the PR is closed. 🔄 Auto-deployed for approved V2 contributors. |
Description of Changes
What was changed
request_timeoutfor the Cucumber/Behave stepI send the API request to the endpoint "{endpoint}".requests.post(..., timeout=request_timeout)with a default of 60 seconds ifcontext.request_timeoutis not set.Why the change was made
split-pdf-by-sectionsbecause the step previously performed a blockingrequests.post(...)without a timeout./api/v1/general/split-pdf-by-sectionsendpoint stalls or takes excessively long in the container, the test never progresses and the CI eventually aborts the whole job (e.g., “The operation was canceled.”). With a timeout, failures become deterministic and provide actionable error output instead of a stuck pipeline.Checklist
General
Documentation
Translations (if applicable)
scripts/counter_translation.pyUI Changes (if applicable)
Testing (if applicable)