Strict mode error is now 400 instead of 403 #6560
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/openapi/test_strictmode.py (1)
1690-1922: Consider adding explicit status code assertionsWhile the error message assertions correctly validate the removal of the "Forbidden:" prefix, adding explicit status code checks would ensure the complete validation of the error categorization change from 403 to 400.
Example implementation:
response = request_with_validation(...) assert not response.ok +assert response.status_code == 400 assert "Limit exceeded 30 > 15 for \"limit\"" in response.json()['status']['error']This pattern could be applied to all the error response tests to explicitly validate that the HTTP status code is 400 (Bad Request) rather than 403 (Forbidden).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/openapi/test_strictmode.py(7 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (13)
- GitHub Check: test-shard-snapshot-api-s3-minio
- GitHub Check: test-snapshot-operations-s3-minio
- GitHub Check: test-low-resources
- GitHub Check: test-consistency
- GitHub Check: test-consensus-compose
- GitHub Check: integration-tests
- GitHub Check: Basic TLS/HTTPS tests
- GitHub Check: integration-tests-consensus
- GitHub Check: rust-tests (windows-latest)
- GitHub Check: rust-tests (ubuntu-latest)
- GitHub Check: rust-tests (macos-latest)
- GitHub Check: lint
- GitHub Check: storage-compat-test
🔇 Additional comments (7)
tests/openapi/test_strictmode.py (7)
1691-1691: LGTM! The error message no longer includes "Forbidden:" prefix.The assertion has been updated to match the new error message format after the change from 403 Forbidden to 400 Bad Request.
1706-1706: LGTM! The error message no longer includes "Forbidden:" prefix.The assertion has been updated to match the new error message format after the change from 403 Forbidden to 400 Bad Request.
1736-1736: LGTM! The error message no longer includes "Forbidden:" prefix.The assertion has been updated to match the new error message format after the change from 403 Forbidden to 400 Bad Request.
1863-1863: LGTM! The error message no longer includes "Forbidden:" prefix.The assertion has been updated to match the new error message format after the change from 403 Forbidden to 400 Bad Request.
1878-1878: LGTM! The error message no longer includes "Forbidden:" prefix.The assertion has been updated to match the new error message format after the change from 403 Forbidden to 400 Bad Request.
1907-1907: LGTM! The error message no longer includes "Forbidden:" prefix.The assertion has been updated to match the new error message format after the change from 403 Forbidden to 400 Bad Request.
1922-1922: LGTM! The error message no longer includes "Forbidden:" prefix.The assertion has been updated to match the new error message format after the change from 403 Forbidden to 400 Bad Request.
* strcit mode error is now 400 instead of 403 * fix test
* strcit mode error is now 400 instead of 403 * fix test
Fixes #6555