Template the requestAttributesEnabled parameter for the AccessLogValve - #27655
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughSummaryThis pull request templates the Changes
ImpactOperators can now control whether request attributes are included in HTTP access logs by changing the WalkthroughA new configuration property 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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.
🧹 Nitpick comments (1)
modules/distribution/src/repository/resources/conf/templates/repository/conf/tomcat/catalina-server.xml.j2 (1)
137-140: Boolean value will render as Python-styleTrue/False.Jinja2 renders Python booleans with capitalized first letter, so
{{http_access_log.request_attributes_enabled}}will emitFalse(orTrue) rather than the XML-idiomatic lowercasefalse/true. WhileBoolean.parseBooleanin Tomcat is case-insensitive and will still resolve correctly, the rendered XML is inconsistent with standard conventions and other boolean attributes. Consider normalizing to lowercase.Proposed fix
- requestAttributesEnabled="{{http_access_log.request_attributes_enabled}}" + requestAttributesEnabled="{{ http_access_log.request_attributes_enabled | string | lower }}"Jinja2 rendering of Python boolean False in template output🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@modules/distribution/src/repository/resources/conf/templates/repository/conf/tomcat/catalina-server.xml.j2` around lines 137 - 140, The template emits Python-style booleans for the Valve attribute; update the catalina-server.xml.j2 Valve attribute for http_access_log.request_attributes_enabled so it is rendered in lowercase (standard "true"/"false") by applying Jinja2's lowercase filter/normalization to the variable (http_access_log.request_attributes_enabled) so the rendered XML uses lowercase boolean values.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@modules/distribution/src/repository/resources/conf/templates/repository/conf/tomcat/catalina-server.xml.j2`:
- Around line 137-140: The template emits Python-style booleans for the Valve
attribute; update the catalina-server.xml.j2 Valve attribute for
http_access_log.request_attributes_enabled so it is rendered in lowercase
(standard "true"/"false") by applying Jinja2's lowercase filter/normalization to
the variable (http_access_log.request_attributes_enabled) so the rendered XML
uses lowercase boolean values.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 52f858ca-c021-43c2-8a5e-827a5ae88a57
📒 Files selected for processing (2)
modules/distribution/src/repository/resources/conf/default.jsonmodules/distribution/src/repository/resources/conf/templates/repository/conf/tomcat/catalina-server.xml.j2
ad24cc7 to
3411395
Compare
|
|
PR builder started |
|
PR builder completed |
jenkins-is-staging
left a comment
There was a problem hiding this comment.
Approving the pull request based on the successful pr build https://github.com/wso2/product-is/actions/runs/26809743574



Purpose
Related Issue