-
Notifications
You must be signed in to change notification settings - Fork 57
LCORE-973: CORS configuration #845
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
LCORE-973: CORS configuration #845
Conversation
WalkthroughAdded explicit CORS fields to the configuration model and updated ServiceConfiguration to initialize CORS via a default factory; expanded TLS documentation and enriched CORS docs/artifacts. Documentation and a diagram/schema artifact were updated; one diagram widened CORS field nullability. Changes
Sequence Diagram(s)(No sequence diagram — changes are data-model and documentation updates without control-flow modifications.) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/models/config.py (1)
225-231: Fix type-checking error forcorsdefault_factorypyright and mypy complain because
Field(default_factory=CORSConfiguration)passes the class object where the type stubs expect a callable returning the value. At runtime this works, but static checkers don’t treat the class as a compatibleCallable.Wrap it in a zero-arg lambda (or helper function) to satisfy both tools:
class ServiceConfiguration(ConfigurationBase): @@ - tls_config: TLSConfiguration = Field( - default_factory=lambda: TLSConfiguration( - tls_certificate_path=None, tls_key_path=None, tls_key_password=None - ) - ) - cors: CORSConfiguration = Field(default_factory=CORSConfiguration) + tls_config: TLSConfiguration = Field( + default_factory=lambda: TLSConfiguration( + tls_certificate_path=None, tls_key_path=None, tls_key_password=None + ) + ) + cors: CORSConfiguration = Field(default_factory=lambda: CORSConfiguration())This aligns with the pydantic v2
Fieldtype hints and should clear the pyright/mypy failures.
🧹 Nitpick comments (1)
src/models/config.py (1)
92-122: CORSConfiguration fields look correct; consider small doc/message tweaksThe new public fields and defaults line up well with FastAPI’s
CORSMiddlewaresemantics, and usinglist[str]with["*"]defaults is appropriate for a configuration model. A couple of minor polish points:
- It might be helpful to explicitly mention in
allow_origins’ description that"*"is a wildcard for all origins (parallel to how you explain"*"for methods/headers).- In the validator error message below (Lines 129-134), there is a small grammatical issue:
"disable credential."→"disable credentials.".Example tweak:
allow_origins: list[str] = Field( ["*"], title="Allow origins", - description="An origin is the combination of protocol (http, https), " - "domain (myapp.com, localhost, localhost.tiangolo.com), and port (80, " - "443, 8080).", + description="A list of origins allowed for cross-origin requests. An origin " + "is the combination of protocol (http, https), domain " + "(myapp.com, localhost, localhost.tiangolo.com), and port (80, 443, 8080). " + "Use ['*'] to allow all origins.", )and
- "allow origins contains '*' wildcard." - "Use explicit origins or disable credential." + "allow_origins contains the '*' wildcard. " + "Use explicit origins or disable credentials."
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/models/config.py(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
src/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.py: Use absolute imports for internal modules in LCS project (e.g.,from auth import get_auth_dependency)
All modules must start with descriptive docstrings explaining their purpose
Uselogger = logging.getLogger(__name__)pattern for module logging
All functions must include complete type annotations for parameters and return types, using modern syntax (str | int) andOptional[Type]orType | None
All functions must have docstrings with brief descriptions following Google Python docstring conventions
Function names must use snake_case with descriptive, action-oriented names (get_, validate_, check_)
Avoid in-place parameter modification anti-patterns; return new data structures instead of modifying input parameters
Useasync deffor I/O operations and external API calls
All classes must include descriptive docstrings explaining their purpose following Google Python docstring conventions
Class names must use PascalCase with descriptive names and standard suffixes:Configurationfor config classes,Error/Exceptionfor exceptions,Resolverfor strategy patterns,Interfacefor abstract base classes
Abstract classes must use ABC with@abstractmethoddecorators
Include complete type annotations for all class attributes in Python classes
Useimport loggingand module logger pattern with standard log levels: debug, info, warning, error
Files:
src/models/config.py
src/models/config.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/models/config.py: All configuration must use Pydantic models extendingConfigurationBasewithextra="forbid"to reject unknown fields
Use type hintsOptional[FilePath],PositiveInt,SecretStrfor Pydantic configuration models
Files:
src/models/config.py
src/models/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/models/**/*.py: Use@field_validatorand@model_validatorfor custom validation in Pydantic models
Pydantic configuration classes must extendConfigurationBase; data models must extendBaseModel
Files:
src/models/config.py
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: lightspeed-core/lightspeed-stack PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:58:04.410Z
Learning: Applies to src/models/config.py : All configuration must use Pydantic models extending `ConfigurationBase` with `extra="forbid"` to reject unknown fields
🪛 GitHub Actions: Pydocstyle
src/models/config.py
[error] 39-39: pydocstyle: D412 violation in TLSConfiguration. No blank lines allowed between a section header and its content ('See Also').
[error] 79-79: pydocstyle: D412 violation in CORSConfiguration. No blank lines allowed between a section header and its content ('See Also').
🪛 GitHub Actions: Pyright
src/models/config.py
[error] 230-230: Pyright type error: Argument of type "type[CORSConfiguration]" cannot be assigned to parameter "default_factory" of type "(() -> _T@Field) | ((dict[str, Any]) -> _T@Field)" in function "Field". This indicates a mismatch in default_factory usage for a Pydantic Field at config.py:230.
🪛 GitHub Actions: Type checks
src/models/config.py
[error] 230-230: Command 'uv run mypy --explicit-package-bases --disallow-untyped-calls --disallow-untyped-defs --disallow-incomplete-defs --ignore-missing-imports --disable-error-code attr-defined src/' failed with exit code 1. mypy error: Argument 'default_factory' to 'Field' has incompatible type 'type[CORSConfiguration]'; expected 'Callable[[], Never] | Callable[[dict[str, Any]], Never]'.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: build-pr
- GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-on-pull-request
- GitHub Check: e2e_tests (ci)
- GitHub Check: e2e_tests (azure)
0adde76 to
44b5076
Compare
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.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/models/config.py (1)
125-132: Fix spacing and wording in CORS validation error messageThe concatenated literals currently produce
"wildcard.Use explicit origins..."(missing space after the period) and use the singular "credential". Consider tightening this up:- raise ValueError( - "Invalid CORS configuration: allow_credentials can not be set to true when " - "allow origins contains '*' wildcard." - "Use explicit origins or disable credential." - ) + raise ValueError( + "Invalid CORS configuration: allow_credentials cannot be set to true when " + "allow_origins contains '*' wildcard. " + "Use explicit origins or disable credentials." + )This keeps the behavior the same while improving the user‑facing message.
🧹 Nitpick comments (1)
src/models/config.py (1)
228-235: Simplifycorsdefault_factory to rely on CORSConfiguration defaultsSince
CORSConfigurationalready defines defaults forallow_origins,allow_credentials,allow_methods, andallow_headers, the lambda that re‑specifies the same values is redundant and slightly harder to maintain. You can simplify to:- cors: CORSConfiguration = Field( - default_factory=lambda: CORSConfiguration( - allow_origins=["*"], - allow_credentials=False, - allow_methods=["*"], - allow_headers=["*"], - ) - ) + cors: CORSConfiguration = Field(default_factory=CORSConfiguration)This keeps the effective defaults identical while avoiding duplication and making future changes to CORSConfiguration’s defaults automatically flow through.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/models/config.py(3 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
src/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.py: Use absolute imports for internal modules in LCS project (e.g.,from auth import get_auth_dependency)
All modules must start with descriptive docstrings explaining their purpose
Uselogger = logging.getLogger(__name__)pattern for module logging
All functions must include complete type annotations for parameters and return types, using modern syntax (str | int) andOptional[Type]orType | None
All functions must have docstrings with brief descriptions following Google Python docstring conventions
Function names must use snake_case with descriptive, action-oriented names (get_, validate_, check_)
Avoid in-place parameter modification anti-patterns; return new data structures instead of modifying input parameters
Useasync deffor I/O operations and external API calls
All classes must include descriptive docstrings explaining their purpose following Google Python docstring conventions
Class names must use PascalCase with descriptive names and standard suffixes:Configurationfor config classes,Error/Exceptionfor exceptions,Resolverfor strategy patterns,Interfacefor abstract base classes
Abstract classes must use ABC with@abstractmethoddecorators
Include complete type annotations for all class attributes in Python classes
Useimport loggingand module logger pattern with standard log levels: debug, info, warning, error
Files:
src/models/config.py
src/models/config.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/models/config.py: All configuration must use Pydantic models extendingConfigurationBasewithextra="forbid"to reject unknown fields
Use type hintsOptional[FilePath],PositiveInt,SecretStrfor Pydantic configuration models
Files:
src/models/config.py
src/models/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/models/**/*.py: Use@field_validatorand@model_validatorfor custom validation in Pydantic models
Pydantic configuration classes must extendConfigurationBase; data models must extendBaseModel
Files:
src/models/config.py
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: lightspeed-core/lightspeed-stack PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:58:04.410Z
Learning: Applies to src/models/config.py : All configuration must use Pydantic models extending `ConfigurationBase` with `extra="forbid"` to reject unknown fields
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: build-pr
- GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-on-pull-request
- GitHub Check: e2e_tests (ci)
- GitHub Check: e2e_tests (azure)
🔇 Additional comments (2)
src/models/config.py (2)
39-50: TLSConfiguration docstring and 'See also' section look goodThe expanded TLS explanation and the adjusted “See also:” section (with bullets directly under the header) read well and should satisfy the earlier D412 complaint; no further change needed here. Please just re-run pydocstyle to confirm the rule is now green.
78-120: CORSConfiguration documentation and field definitions are clear and schema‑consistentThe enriched CORSConfiguration docstring and the new
allow_origins/allow_credentials/allow_methods/allow_headersfields (with FastAPI‑style descriptions and sensible defaults) are consistent with how CORSMiddleware is typically configured, and keeping this model subclassingConfigurationBasemaintains theextra="forbid"contract for config schemas. This is aligned with the configuration modeling guidelines for this module; I don’t see any issues here.As per coding guidelines, this keeps configuration models extending
ConfigurationBasewith forbidden extras.
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.
Actionable comments posted: 2
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
docs/config.html(4 hunks)docs/config.md(3 hunks)docs/config.puml(1 hunks)src/models/config.py(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/models/config.py
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-24T16:58:04.410Z
Learnt from: CR
Repo: lightspeed-core/lightspeed-stack PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:58:04.410Z
Learning: Applies to src/models/config.py : All configuration must use Pydantic models extending `ConfigurationBase` with `extra="forbid"` to reject unknown fields
Applied to files:
docs/config.puml
🪛 markdownlint-cli2 (0.18.1)
docs/config.md
84-84: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
85-85: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
86-86: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
384-384: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
385-385: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
386-386: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: build-pr
- GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-on-pull-request
- GitHub Check: e2e_tests (azure)
- GitHub Check: e2e_tests (ci)
🔇 Additional comments (5)
docs/config.md (1)
91-94: Excellent CORS field documentation.The detailed descriptions for each CORS configuration field are clear, informative, and provide practical guidance including the wildcard syntax.
docs/config.puml (1)
33-36: LGTM - Diagram accurately reflects CORS field optionality.The PlantUML diagram correctly represents the CORSConfiguration fields as Optional types, which aligns with the flexible configuration approach documented in the related files.
docs/config.html (3)
308-322: Comprehensive CORS documentation added.The expanded CORS documentation with explanatory prose and resource links enhances understanding for developers configuring cross-origin resource sharing.
324-328: Well-structured table with helpful descriptions.The addition of the colgroup for column width control and detailed field descriptions makes the CORS configuration table more readable and informative.
Also applies to: 340-363
1000-1015: Valuable TLS documentation enhancement.The expanded TLS overview and resource links provide helpful context for developers implementing HTTPS support.
| CORS or 'Cross-Origin Resource Sharing' refers to the situations when a | ||
| frontend running in a browser has JavaScript code that communicates with a | ||
| backend, and the backend is in a different 'origin' than the frontend. | ||
|
|
||
| Useful resources: | ||
|
|
||
| - [CORS in FastAPI](https://fastapi.tiangolo.com/tutorial/cors/) | ||
| - [Wikipedia article](https://en.wikipedia.org/wiki/Cross-origin_resource_sharing) | ||
| - [What is CORS?](https://dev.to/akshay_chauhan/what-is-cors-explained-8f1) |
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.
Fix list indentation for markdown compliance.
The unordered list items in the "Useful resources" section are indented by 2 spaces. Remove the indentation to comply with MD007.
Apply this diff:
Useful resources:
- - [CORS in FastAPI](https://fastapi.tiangolo.com/tutorial/cors/)
- - [Wikipedia article](https://en.wikipedia.org/wiki/Cross-origin_resource_sharing)
- - [What is CORS?](https://dev.to/akshay_chauhan/what-is-cors-explained-8f1)
+- [CORS in FastAPI](https://fastapi.tiangolo.com/tutorial/cors/)
+- [Wikipedia article](https://en.wikipedia.org/wiki/Cross-origin_resource_sharing)
+- [What is CORS?](https://dev.to/akshay_chauhan/what-is-cors-explained-8f1)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| CORS or 'Cross-Origin Resource Sharing' refers to the situations when a | |
| frontend running in a browser has JavaScript code that communicates with a | |
| backend, and the backend is in a different 'origin' than the frontend. | |
| Useful resources: | |
| - [CORS in FastAPI](https://fastapi.tiangolo.com/tutorial/cors/) | |
| - [Wikipedia article](https://en.wikipedia.org/wiki/Cross-origin_resource_sharing) | |
| - [What is CORS?](https://dev.to/akshay_chauhan/what-is-cors-explained-8f1) | |
| CORS or 'Cross-Origin Resource Sharing' refers to the situations when a | |
| frontend running in a browser has JavaScript code that communicates with a | |
| backend, and the backend is in a different 'origin' than the frontend. | |
| Useful resources: | |
| - [CORS in FastAPI](https://fastapi.tiangolo.com/tutorial/cors/) | |
| - [Wikipedia article](https://en.wikipedia.org/wiki/Cross-origin_resource_sharing) | |
| - [What is CORS?](https://dev.to/akshay_chauhan/what-is-cors-explained-8f1) |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
84-84: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
85-85: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
86-86: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
🤖 Prompt for AI Agents
In docs/config.md around lines 78 to 86, the "Useful resources" unordered list
items are indented by two spaces which violates MD007; remove the leading two
spaces before each list item so the hyphens start at the left margin (no leading
spaces) to make the markdown list top-level and compliant.
| Transport Layer Security (TLS) is a cryptographic protocol designed to | ||
| provide communications security over a computer network, such as the | ||
| Internet. The protocol is widely used in applications such as email, | ||
| instant messaging, and voice over IP, but its use in securing HTTPS remains | ||
| the most publicly visible. | ||
|
|
||
| Useful resources: | ||
|
|
||
| - [FastAPI HTTPS Deployment](https://fastapi.tiangolo.com/deployment/https/) | ||
| - [Transport Layer Security Overview](https://en.wikipedia.org/wiki/Transport_Layer_Security) | ||
| - [What is TLS](https://www.ssltrust.eu/learning/ssl/transport-layer-security-tls) |
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.
Fix list indentation for markdown compliance.
The unordered list items in the "Useful resources" section are indented by 2 spaces. Remove the indentation to comply with MD007.
Apply this diff:
Useful resources:
- - [FastAPI HTTPS Deployment](https://fastapi.tiangolo.com/deployment/https/)
- - [Transport Layer Security Overview](https://en.wikipedia.org/wiki/Transport_Layer_Security)
- - [What is TLS](https://www.ssltrust.eu/learning/ssl/transport-layer-security-tls)
+- [FastAPI HTTPS Deployment](https://fastapi.tiangolo.com/deployment/https/)
+- [Transport Layer Security Overview](https://en.wikipedia.org/wiki/Transport_Layer_Security)
+- [What is TLS](https://www.ssltrust.eu/learning/ssl/transport-layer-security-tls)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Transport Layer Security (TLS) is a cryptographic protocol designed to | |
| provide communications security over a computer network, such as the | |
| Internet. The protocol is widely used in applications such as email, | |
| instant messaging, and voice over IP, but its use in securing HTTPS remains | |
| the most publicly visible. | |
| Useful resources: | |
| - [FastAPI HTTPS Deployment](https://fastapi.tiangolo.com/deployment/https/) | |
| - [Transport Layer Security Overview](https://en.wikipedia.org/wiki/Transport_Layer_Security) | |
| - [What is TLS](https://www.ssltrust.eu/learning/ssl/transport-layer-security-tls) | |
| Transport Layer Security (TLS) is a cryptographic protocol designed to | |
| provide communications security over a computer network, such as the | |
| Internet. The protocol is widely used in applications such as email, | |
| instant messaging, and voice over IP, but its use in securing HTTPS remains | |
| the most publicly visible. | |
| Useful resources: | |
| - [FastAPI HTTPS Deployment](https://fastapi.tiangolo.com/deployment/https/) | |
| - [Transport Layer Security Overview](https://en.wikipedia.org/wiki/Transport_Layer_Security) | |
| - [What is TLS](https://www.ssltrust.eu/learning/ssl/transport-layer-security-tls) |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
384-384: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
385-385: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
386-386: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
🤖 Prompt for AI Agents
In docs/config.md around lines 376 to 386, the "Useful resources" unordered list
items are indented by two spaces which violates MD007; remove the leading
two-space indentation so each list item starts at the beginning of the line (no
leading spaces), keeping the dash and a single space then the link text, to
produce a top-level unordered list that complies with markdown linting.
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.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/unit/models/config/test_cors.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
tests/{unit,integration}/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/{unit,integration}/**/*.py: Use pytest for all unit and integration tests; do not use unittest framework
Unit tests must achieve 60% code coverage; integration tests must achieve 10% coverage
Files:
tests/unit/models/config/test_cors.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Use
pytest-mockwith AsyncMock objects for mocking in tests
Files:
tests/unit/models/config/test_cors.py
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build-pr
- GitHub Check: e2e_tests (azure)
- GitHub Check: e2e_tests (ci)
🔇 Additional comments (1)
tests/unit/models/config/test_cors.py (1)
67-68: I'll help you verify that the error message in the test matches the actual implementation. Let me search for the CORSConfiguration class and its validation logic.
<function_calls>
#!/bin/bashFind CORSConfiguration class definition
fd -t f -e py "cors" | head -20
</function_calls>
<function_calls>
#!/bin/bashSearch for CORSConfiguration class in the codebase
rg -n "class CORSConfiguration" --type=py
</function_calls>
<function_calls>
#!/bin/bashSearch for the error message about allow_credentials and wildcard
rg -n "allow_credentials.*wildcard|allow_origins.*wildcard" --type=py
</function_calls>
<function_calls>
#!/bin/bashLook at the test file to see the context and what it's testing
head -80 tests/unit/models/config/test_cors.py | tail -20
</function_calls>
<function_calls>
#!/bin/bashGet more context from the test file around lines 67-68
sed -n '50,80p' tests/unit/models/config/test_cors.py
</function_calls>
| + "allow_credentials can not be set to true when allow origins contains the '\\*' wildcard." | ||
| + "Use explicit origins or disable credentials." |
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.
Missing space causes sentence concatenation issue.
Line 68 lacks a leading space, causing the two sentences to concatenate without separation: "...wildcard.Use explicit...". This will cause the test to fail unless the actual error message also has this formatting issue.
Apply this diff to add the missing space:
- + "Use explicit origins or disable credentials."
+ + " Use explicit origins or disable credentials."📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| + "allow_credentials can not be set to true when allow origins contains the '\\*' wildcard." | |
| + "Use explicit origins or disable credentials." | |
| "allow_credentials can not be set to true when allow origins contains the '\\*' wildcard." | |
| " Use explicit origins or disable credentials." |
🤖 Prompt for AI Agents
In tests/unit/models/config/test_cors.py around lines 67 to 68, the two string
literals are concatenated without a space resulting in "...wildcard.Use
explicit..." — add a leading space at the start of the second string (or add a
trailing space to the first) so the sentences are separated ("...wildcard. Use
explicit origins or disable credentials.") to match the expected error message.
Description
LCORE-973: CORS configuration
Type of change
Tools used to create PR
Identify any AI code assistants used in this PR (for transparency and review context)
Related Tickets & Documents
Summary by CodeRabbit
New Features
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.