Open
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a new Odoo addon Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📝 Coding Plan
Comment |
tendil
force-pushed
the
18.0-t5246-auditlog_clickhouse-add_module
branch
from
dcc674b to
65ad212
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@auditlog_clickhouse/models/auditlog_log_buffer.py`:
- Around line 300-315: The INSERT and DDL builders interpolate database and
table identifiers unquoted causing breakage and injection risk; add a helper
function named _ch_ident(name: str) that escapes backticks and returns the
identifier wrapped in backticks, then use it when composing SQL in
_insert_rows_to_clickhouse (wrap config.database and the table names
auditlog_log and auditlog_log_line) and in _get_clickhouse_ddl (wrap db_name and
table identifiers); also apply _ch_ident to any dynamically constructed
identifier lists like _CH_LOG_COLUMNS/_CH_LINE_COLUMNS or the column names in
the CREATE TABLE SQL so all identifiers are backtick-quoted before being
interpolated into f-strings.
In `@auditlog_clickhouse/readme/CONFIGURE.md`:
- Around line 30-31: Update the README line that currently reads "Optionally,
click **Create Auditlog Tables** to create the tables and User in the target
database." to remove the claim about creating a user (or explicitly state that
user creation is not performed and must be done beforehand); locate the exact
sentence containing the **Create Auditlog Tables** phrase and either delete "and
User" or replace the sentence with a clarified prerequisite note that the module
only creates tables and that databases/users/grants must be created/provisioned
separately.
In `@auditlog_clickhouse/readme/DESCRIPTION.md`:
- Line 1: Update the opening sentence in DESCRIPTION.md to include the missing
preposition: change "This module implements buffered asynchronous transfers
audit logs from PostgreSQL to ClickHouse." to "This module implements buffered
asynchronous transfers of audit logs from PostgreSQL to ClickHouse." — edit the
first line (the module description/opening sentence) to insert "of" between
"transfers" and "audit logs."
In `@auditlog_clickhouse/tests/test_auditlog_clickhouse.py`:
- Around line 250-351: The TestAuditlogClickhouseConfig class is duplicated;
remove the copy in this file and reference the canonical one to avoid running
tests twice: delete the TestAuditlogClickhouseConfig class here (including its
methods test_01_single_active_on_create, test_02_single_active_on_write,
test_03_test_connection_uses_client,
test_04_queue_channel_field_is_m2o_and_default_is_root,
test_05_onchange_is_active_shows_disclaimer,
test_06_cron_uses_overridden_batch_size_argument) and replace it with a single
import of the canonical TestAuditlogClickhouseConfig from the module that
defines the primary tests (ensuring AuditLogClickhouseCommon and helpers like
create_config and _patched_clickhouse_client remain available), or alternatively
refactor shared test logic into a common base and keep only one concrete
subclass to run.
In `@requirements.txt`:
- Line 2: Update the package name in requirements.txt from the incorrect
underscore form "clickhouse_driver" to the correct PyPI package name
"clickhouse-driver" so pip can install it; locate the line containing
"clickhouse_driver" and replace it with "clickhouse-driver".
---
Duplicate comments:
In `@auditlog_clickhouse/models/auditlog_clickhouse_config.py`:
- Around line 342-391: The DDL built by _get_clickhouse_ddl currently
interpolates identifiers (db_name and table names
auditlog_log/auditlog_log_line) directly into f-strings; update this to use the
same identifier-quoting helper used by _insert_rows_to_clickhouse so identifiers
are consistently escaped/quoted. Locate _get_clickhouse_ddl and replace direct
f-string insertion of db_name and table identifiers with calls to the shared
quoting function (the same helper referenced in _insert_rows_to_clickhouse) so
the CREATE TABLE statements use properly quoted identifiers.
Aldeigja
force-pushed
the
18.0-t5246-auditlog_clickhouse-add_module
branch
from
65ad212 to
c4763c1
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (6)
auditlog_clickhouse/readme/DESCRIPTION.md (1)
1-1:⚠️ Potential issue | 🟡 MinorFix missing preposition in the opening sentence.
Line 1 should read “transfers of audit logs”.
✏️ Proposed fix
-This module implements buffered asynchronous transfers audit logs from PostgreSQL to ClickHouse. +This module implements buffered asynchronous transfers of audit logs from PostgreSQL to ClickHouse.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@auditlog_clickhouse/readme/DESCRIPTION.md` at line 1, Update the opening sentence in the module DESCRIPTION to include the missing preposition "of": change "implements buffered asynchronous transfers audit logs from PostgreSQL to ClickHouse" to "implements buffered asynchronous transfers of audit logs from PostgreSQL to ClickHouse" (locate the sentence in the README/description text where the phrase "buffered asynchronous transfers audit logs" appears).requirements.txt (1)
2-2:⚠️ Potential issue | 🟠 MajorUse PyPI package name
clickhouse-driverin requirements.txt.Line 2 uses the import/module name, which pip won’t resolve. Switch to the canonical PyPI package name to avoid install failures.
🛠️ Proposed fix
-clickhouse_driver +clickhouse-driverWhat is the correct PyPI package name to install the ClickHouse Python driver used as `clickhouse_driver` in imports?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@requirements.txt` at line 2, The requirements.txt entry uses the module/import name "clickhouse_driver" which pip won't resolve; update the dependency to the canonical PyPI package name "clickhouse-driver" by replacing the "clickhouse_driver" line in requirements.txt so installs succeed when importing clickhouse_driver in code.auditlog_clickhouse/readme/CONFIGURE.md (1)
30-31:⚠️ Potential issue | 🟡 MinorRemove the user-creation claim.
Line 31 still says the action creates a user, which contradicts Line 5. Please remove the user-creation wording to keep prerequisites consistent.
✏️ Suggested wording fix
- - Optionally, click **Create Auditlog Tables** to create the tables and User in the target database. + - Optionally, click **Create Auditlog Tables** to create the tables in the target database.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@auditlog_clickhouse/readme/CONFIGURE.md` around lines 30 - 31, Remove the incorrect "and User" claim from the sentence that reads "Optionally, click **Create Auditlog Tables** to create the tables and User in the target database" so it only mentions creating tables (e.g., change to "Optionally, click **Create Auditlog Tables** to create the tables in the target database"); update the wording to match the prerequisite statement on Line 5 and ensure no mention remains that a user is created.auditlog_clickhouse/tests/test_auditlog_clickhouse.py (1)
250-351: 🧹 Nitpick | 🔵 TrivialDeduplicate TestAuditlogClickhouseConfig to avoid running the same suite twice.
This class overlaps with the standalone config tests and adds maintenance/runtime cost.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@auditlog_clickhouse/tests/test_auditlog_clickhouse.py` around lines 250 - 351, The test class TestAuditlogClickhouseConfig duplicates existing standalone config tests (it subclasses AuditLogClickhouseCommon) and should be removed or merged to avoid running the same suite twice; delete this entire TestAuditlogClickhouseConfig class (or consolidate its unique tests into the existing config test class) so only one test class referencing AuditLogClickhouseCommon remains, ensuring no duplicate test collection for the same scenarios.auditlog_clickhouse/models/auditlog_log_buffer.py (1)
300-328:⚠️ Potential issue | 🟠 MajorQuote database/table identifiers in INSERT statements.
Unquoted identifiers break on reserved words/special characters and can be exploited if configuration values are untrusted.🔒 Suggested fix (identifier quoting)
+ `@staticmethod` + def _ch_ident(name: str) -> str: + return f"`{name.replace('`', '``')}`" + def _insert_rows_to_clickhouse( self, client, config, log_rows, line_rows, valid_buffers ): try: if log_rows: + db = self._ch_ident(config.database) + log_tbl = self._ch_ident("auditlog_log") client.execute( - f"INSERT INTO {config.database}.auditlog_log (" + f"INSERT INTO {db}.{log_tbl} (" f"{', '.join(self._CH_LOG_COLUMNS)}) VALUES", log_rows, ) if line_rows: + db = self._ch_ident(config.database) + line_tbl = self._ch_ident("auditlog_log_line") client.execute( - f"INSERT INTO {config.database}.auditlog_log_line (" + f"INSERT INTO {db}.{line_tbl} (" f"{', '.join(self._CH_LINE_COLUMNS)}) VALUES", line_rows, )Based on learnings: In the auditlog_clickhouse module, when constructing ClickHouse queries, quote database/table identifiers (using backticks or double quotes) instead of relying on regex-based validation. This ensures proper handling of special characters and reserved words. Apply this pattern to all Python files under auditlog_clickhouse (e.g., auditlog_clickhouse/**/*.py) that build queries or reference identifiers, ensuring identifiers are properly quoted and escaping is consistent.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@auditlog_clickhouse/models/auditlog_log_buffer.py` around lines 300 - 328, The INSERT statements in _insert_rows_to_clickhouse construct unquoted identifiers using config.database and column lists (_CH_LOG_COLUMNS / _CH_LINE_COLUMNS), which breaks on reserved words/special chars and is unsafe; update the INSERTs to quote/escape the database and table identifiers (e.g., wrap {config.database}.auditlog_log and {config.database}.auditlog_log_line in ClickHouse identifier quotes such as backticks or double quotes and ensure any quote chars in config.database are escaped), and similarly ensure the column names in _CH_LOG_COLUMNS and _CH_LINE_COLUMNS are properly quoted/escaped before joining so all generated SQL is safe and robust.auditlog_clickhouse/models/auditlog_clickhouse_config.py (1)
342-392:⚠️ Potential issue | 🟠 MajorQuote database and table identifiers in DDL.
Identifiers are interpolated without quoting, which breaks on reserved words/special characters and risks injection if configuration is untrusted.🔒 Suggested fix (identifier quoting)
+ `@staticmethod` + def _ch_ident(name: str) -> str: + return f"`{name.replace('`', '``')}`" + def _get_clickhouse_ddl(self) -> list[str]: """ Return ClickHouse DDL statements for required objects. @@ - db_name = self.database + db_name = self._ch_ident(self.database) + log_tbl = self._ch_ident("auditlog_log") + line_tbl = self._ch_ident("auditlog_log_line") @@ - CREATE TABLE IF NOT EXISTS {db_name}.auditlog_log + CREATE TABLE IF NOT EXISTS {db_name}.{log_tbl} @@ - CREATE TABLE IF NOT EXISTS {db_name}.auditlog_log_line + CREATE TABLE IF NOT EXISTS {db_name}.{line_tbl}Based on learnings: In the auditlog_clickhouse module, when constructing ClickHouse queries, quote database/table identifiers (using backticks or double quotes) instead of relying on regex-based validation. This ensures proper handling of special characters and reserved words. Apply this pattern to all Python files under auditlog_clickhouse (e.g., auditlog_clickhouse/**/*.py) that build queries or reference identifiers, ensuring identifiers are properly quoted and escaping is consistent.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@auditlog_clickhouse/models/auditlog_clickhouse_config.py` around lines 342 - 392, The SQL DDL built in _get_clickhouse_ddl currently interpolates the database and table names (db_name, auditlog_log, auditlog_log_line) without quoting, which breaks on reserved words/special chars and allows injection; fix by introducing and using a quoting helper (e.g., quote_identifier(name)) and replace direct interpolation of db_name and table identifiers in _get_clickhouse_ddl with the quoted versions (ensure the helper consistently escapes/backticks or double-quotes according to ClickHouse rules and use it for every identifier used in the f-strings).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@auditlog_clickhouse/data/auditlog_clickhouse_queue.xml`:
- Around line 13-18: Update the action title for the record with id
"action_configure_auditlog_clickhouse_cron" (model "ir.actions.act_window") to a
clearer label reflecting its purpose—e.g., change the <field name="name"> value
from "Edit export action" to something like "Configure ClickHouse flush cron" or
"Edit ClickHouse flush schedule"—so the UI clearly indicates this action opens
the ClickHouse flush cron configuration.
In `@auditlog_clickhouse/models/auditlog_rule.py`:
- Around line 178-199: The base_log construction in
_get_audit_model_id/_get_rule_settings (function building base_log) discards
request/session metadata by hardcoding "http_request_id" and "http_session_id"
to None; instead read these keys from additional_log_values (e.g.,
additional_log_values.get("http_request_id") and .get("http_session_id")) and
use those values in base_log while preserving fallback to None if absent,
ensuring additional_log_values is used when present and log_type is still taken
from additional_log_values.
---
Duplicate comments:
In `@auditlog_clickhouse/models/auditlog_clickhouse_config.py`:
- Around line 342-392: The SQL DDL built in _get_clickhouse_ddl currently
interpolates the database and table names (db_name, auditlog_log,
auditlog_log_line) without quoting, which breaks on reserved words/special chars
and allows injection; fix by introducing and using a quoting helper (e.g.,
quote_identifier(name)) and replace direct interpolation of db_name and table
identifiers in _get_clickhouse_ddl with the quoted versions (ensure the helper
consistently escapes/backticks or double-quotes according to ClickHouse rules
and use it for every identifier used in the f-strings).
In `@auditlog_clickhouse/models/auditlog_log_buffer.py`:
- Around line 300-328: The INSERT statements in _insert_rows_to_clickhouse
construct unquoted identifiers using config.database and column lists
(_CH_LOG_COLUMNS / _CH_LINE_COLUMNS), which breaks on reserved words/special
chars and is unsafe; update the INSERTs to quote/escape the database and table
identifiers (e.g., wrap {config.database}.auditlog_log and
{config.database}.auditlog_log_line in ClickHouse identifier quotes such as
backticks or double quotes and ensure any quote chars in config.database are
escaped), and similarly ensure the column names in _CH_LOG_COLUMNS and
_CH_LINE_COLUMNS are properly quoted/escaped before joining so all generated SQL
is safe and robust.
In `@auditlog_clickhouse/readme/CONFIGURE.md`:
- Around line 30-31: Remove the incorrect "and User" claim from the sentence
that reads "Optionally, click **Create Auditlog Tables** to create the tables
and User in the target database" so it only mentions creating tables (e.g.,
change to "Optionally, click **Create Auditlog Tables** to create the tables in
the target database"); update the wording to match the prerequisite statement on
Line 5 and ensure no mention remains that a user is created.
In `@auditlog_clickhouse/readme/DESCRIPTION.md`:
- Line 1: Update the opening sentence in the module DESCRIPTION to include the
missing preposition "of": change "implements buffered asynchronous transfers
audit logs from PostgreSQL to ClickHouse" to "implements buffered asynchronous
transfers of audit logs from PostgreSQL to ClickHouse" (locate the sentence in
the README/description text where the phrase "buffered asynchronous transfers
audit logs" appears).
In `@auditlog_clickhouse/tests/test_auditlog_clickhouse.py`:
- Around line 250-351: The test class TestAuditlogClickhouseConfig duplicates
existing standalone config tests (it subclasses AuditLogClickhouseCommon) and
should be removed or merged to avoid running the same suite twice; delete this
entire TestAuditlogClickhouseConfig class (or consolidate its unique tests into
the existing config test class) so only one test class referencing
AuditLogClickhouseCommon remains, ensuring no duplicate test collection for the
same scenarios.
In `@requirements.txt`:
- Line 2: The requirements.txt entry uses the module/import name
"clickhouse_driver" which pip won't resolve; update the dependency to the
canonical PyPI package name "clickhouse-driver" by replacing the
"clickhouse_driver" line in requirements.txt so installs succeed when importing
clickhouse_driver in code.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
auditlog_clickhouse/security/ir.model.access.csvis excluded by!**/*.csv
📒 Files selected for processing (22)
auditlog_clickhouse/README.rstauditlog_clickhouse/__init__.pyauditlog_clickhouse/__manifest__.pyauditlog_clickhouse/data/auditlog_clickhouse_queue.xmlauditlog_clickhouse/i18n/auditlog_clickhouse.potauditlog_clickhouse/models/__init__.pyauditlog_clickhouse/models/auditlog_clickhouse_config.pyauditlog_clickhouse/models/auditlog_log_buffer.pyauditlog_clickhouse/models/auditlog_rule.pyauditlog_clickhouse/models/clickhouse_client.pyauditlog_clickhouse/pyproject.tomlauditlog_clickhouse/readme/CONFIGURE.mdauditlog_clickhouse/readme/CONTEXT.mdauditlog_clickhouse/readme/DESCRIPTION.mdauditlog_clickhouse/readme/USAGE.mdauditlog_clickhouse/static/description/index.htmlauditlog_clickhouse/tests/__init__.pyauditlog_clickhouse/tests/common.pyauditlog_clickhouse/tests/test_auditlog_clickhouse.pyauditlog_clickhouse/tests/test_clickhouse_config.pyauditlog_clickhouse/views/auditlog_clickhouse_config_views.xmlrequirements.txt
tendil
force-pushed
the
18.0-t5246-auditlog_clickhouse-add_module
branch
from
c4763c1 to
6b00222
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (8)
requirements.txt (1)
2-2:⚠️ Potential issue | 🟠 MajorUse the PyPI package name
clickhouse-driver.Line 2 uses
clickhouse_driver, which pip won’t resolve; the published package uses the hyphenated name.🧩 Suggested fix
-clickhouse_driver +clickhouse-driver🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@requirements.txt` at line 2, Replace the incorrect package name "clickhouse_driver" in requirements.txt with the correct PyPI package name "clickhouse-driver" so pip can resolve and install it; update the entry (the second line referencing clickhouse_driver) to use the hyphenated form.auditlog_clickhouse/readme/DESCRIPTION.md (1)
1-1:⚠️ Potential issue | 🟡 MinorFix missing preposition in the opening sentence.
Line 1 reads “transfers audit logs”; it should be “transfers of audit logs.”
✏️ Suggested fix
-This module implements buffered asynchronous transfers audit logs from PostgreSQL to ClickHouse. +This module implements buffered asynchronous transfers of audit logs from PostgreSQL to ClickHouse.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@auditlog_clickhouse/readme/DESCRIPTION.md` at line 1, Update the opening sentence in DESCRIPTION.md: replace the phrase "This module implements buffered asynchronous transfers audit logs from PostgreSQL to ClickHouse." with "This module implements buffered asynchronous transfers of audit logs from PostgreSQL to ClickHouse." to add the missing preposition "of" in the sentence.auditlog_clickhouse/data/auditlog_clickhouse_queue.xml (1)
13-26: Clarify the action title.The action name "Edit export action" is ambiguous. Consider a more descriptive label like "Configure ClickHouse flush schedule" to clearly indicate its purpose.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@auditlog_clickhouse/data/auditlog_clickhouse_queue.xml` around lines 13 - 26, Update the record with id "action_configure_auditlog_clickhouse_cron" so its <field name="name"> uses a clearer, descriptive label (e.g., "Configure ClickHouse flush schedule") instead of "Edit export action"; locate the record by id and/or by res_model="ir.cron" and res_id ref="auditlog_clickhouse.ir_cron_auditlog_clickhouse_enqueue_flush" and replace the name text accordingly to reflect that this action configures the ClickHouse enqueue/flush schedule.auditlog_clickhouse/readme/CONFIGURE.md (1)
30-31: Contradictory user creation claim.Line 31 states "create the tables and User" but line 5 explicitly says the module does not create users/grants. Remove "and User" to resolve the contradiction.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@auditlog_clickhouse/readme/CONFIGURE.md` around lines 30 - 31, Update the CONFIGURE.md text to remove the incorrect claim that the module creates a database user: in the bullet that currently reads "Optionally, click **Create Auditlog Tables** to create the tables and User in the target database," delete "and User" so it only states the tables are created; keep the rest of the wording and ensure the "Create Auditlog Tables" phrase remains as-is to match the UI label.auditlog_clickhouse/models/auditlog_log_buffer.py (1)
300-315:⚠️ Potential issue | 🟠 MajorQuote ClickHouse identifiers in INSERTs.
Line 306–313 interpolate
{config.database}and table names without quoting, which breaks on reserved words/special chars and is unsafe if config inputs are not fully trusted. Prefer a shared_ch_ident()helper and use it for db/table identifiers.🛠️ Proposed fix
+ `@staticmethod` + def _ch_ident(name: str) -> str: + # ClickHouse supports backtick quoting; escape backticks inside names. + return f"`{name.replace('`', '``')}`" + def _insert_rows_to_clickhouse( self, client, config, log_rows, line_rows, valid_buffers ): try: + db = self._ch_ident(config.database) if log_rows: + log_tbl = self._ch_ident("auditlog_log") client.execute( - f"INSERT INTO {config.database}.auditlog_log (" + f"INSERT INTO {db}.{log_tbl} (" f"{', '.join(self._CH_LOG_COLUMNS)}) VALUES", log_rows, ) if line_rows: + line_tbl = self._ch_ident("auditlog_log_line") client.execute( - f"INSERT INTO {config.database}.auditlog_log_line (" + f"INSERT INTO {db}.{line_tbl} (" f"{', '.join(self._CH_LINE_COLUMNS)}) VALUES", line_rows, )#!/bin/bash # Find other ClickHouse SQL statements that may need identifier quoting rg -n "INSERT INTO|CREATE TABLE IF NOT EXISTS" auditlog_clickhouse/models -g '*.py'Based on learnings: In the auditlog_clickhouse module, when constructing ClickHouse queries, quote database/table identifiers (using backticks or double quotes) instead of relying on regex-based validation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@auditlog_clickhouse/models/auditlog_log_buffer.py` around lines 300 - 315, The INSERT statements in _insert_rows_to_clickhouse currently interpolate unquoted identifiers (config.database and table names) which can break on reserved words or special chars; update the method to use the shared identifier-quoting helper (e.g., _ch_ident) to produce safe identifiers for the database and table (replace occurrences like f"{config.database}.auditlog_log" and f"{config.database}.auditlog_log_line" with the helper output), leaving the column lists (_CH_LOG_COLUMNS, _CH_LINE_COLUMNS) and values (log_rows, line_rows) unchanged so the client.execute calls use quoted identifiers.auditlog_clickhouse/models/auditlog_clickhouse_config.py (1)
342-392:⚠️ Potential issue | 🟠 MajorQuote database/table identifiers in DDL.
Line 354 and Line 375 interpolate
{db_name}directly into DDL. This can break on reserved words/special characters and is unsafe if names are ever untrusted. Use a quoting helper.🛠️ Proposed fix
+ `@staticmethod` + def _ch_ident(name: str) -> str: + # ClickHouse supports backtick quoting; escape backticks inside names. + return f"`{name.replace('`', '``')}`" + def _get_clickhouse_ddl(self) -> list[str]: """ Return ClickHouse DDL statements for required objects. @@ - db_name = self.database + db_name = self._ch_ident(self.database) + log_tbl = self._ch_ident("auditlog_log") + line_tbl = self._ch_ident("auditlog_log_line") return [ f""" - CREATE TABLE IF NOT EXISTS {db_name}.auditlog_log + CREATE TABLE IF NOT EXISTS {db_name}.{log_tbl} @@ """, f""" - CREATE TABLE IF NOT EXISTS {db_name}.auditlog_log_line + CREATE TABLE IF NOT EXISTS {db_name}.{line_tbl}Based on learnings: In the auditlog_clickhouse module, when constructing ClickHouse queries, quote database/table identifiers (using backticks or double quotes) instead of relying on regex-based validation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@auditlog_clickhouse/models/auditlog_clickhouse_config.py` around lines 342 - 392, The DDL in _get_clickhouse_ddl interpolates db_name directly which can break on special/reserved names—replace direct interpolation with a quoting helper (e.g., quote_identifier) and use it for the database and table identifiers in both returned CREATE TABLE strings (replace occurrences like {db_name}.auditlog_log and {db_name}.auditlog_log_line with quoted forms), ensuring the helper emits ClickHouse-safe quoted identifiers (backticks or double quotes) and is used consistently in that function.auditlog_clickhouse/tests/test_auditlog_clickhouse.py (1)
250-351: 🧹 Nitpick | 🔵 TrivialAvoid duplicate
TestAuditlogClickhouseConfigdefinitions.If a canonical config test class already exists (e.g., in a dedicated config test module), keep a single concrete class to avoid running the same tests twice and drifting expectations.
#!/bin/bash # Locate duplicate TestAuditlogClickhouseConfig definitions rg -n "class TestAuditlogClickhouseConfig" -g '*.py'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@auditlog_clickhouse/tests/test_auditlog_clickhouse.py` around lines 250 - 351, This file defines a concrete test class TestAuditlogClickhouseConfig that duplicates other canonical config tests; remove or convert this duplicate so tests aren't run twice: either delete this TestAuditlogClickhouseConfig class entirely or make it non-concrete (e.g., rename to a different non-conflicting class or inherit/mark it abstract) and ensure only the intended concrete subclass of AuditLogClickhouseCommon remains, then run the grep from the review to verify a single TestAuditlogClickhouseConfig definition across the codebase.auditlog_clickhouse/models/auditlog_rule.py (1)
188-199:⚠️ Potential issue | 🟠 MajorPreserve request/session IDs from
additional_log_values.Line 194–195 hard-code
None, so any request/session IDs passed in are dropped, which breaks traceability.🛠️ Proposed fix
- "http_request_id": None, - "http_session_id": None, + "http_request_id": additional_log_values.get("http_request_id"), + "http_session_id": additional_log_values.get("http_session_id"),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@auditlog_clickhouse/models/auditlog_rule.py` around lines 188 - 199, The base_log dict currently hard-codes "http_request_id" and "http_session_id" to None, dropping any values in additional_log_values; update the assignments in the base_log construction (the dict in auditlog_rule.py that contains "model_id", "model_name", etc.) to use additional_log_values.get("http_request_id") and additional_log_values.get("http_session_id") (with a fallback to None) so any provided request/session IDs are preserved when building the log payload.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@auditlog_clickhouse/models/clickhouse_client.py`:
- Around line 7-10: The import block currently catches a broad Exception when
importing clickhouse_driver; narrow this to except ImportError (or except
(ImportError, ModuleNotFoundError) if you prefer) so only import failures are
swallowed—modify the try/except around "from clickhouse_driver import Client as
ClickHouseClient" to catch ImportError and set ClickHouseClient = None in that
except block.
In `@auditlog_clickhouse/readme/CONTEXT.md`:
- Around line 1-5: Replace the phrase "grow without limits" with a more precise
wording such as "grow indefinitely" (or "grow unbounded") in the CONTEXT.md text
and adjust the bullet punctuation so the first two items end with semicolons and
the final item ends with a period; specifically update the line containing "grow
without limits" and the three bullets ("Database bloat;", "Immutability gap:
...;", "Performance overhead: ...") to use the clarified wording and proper
terminal punctuation.
---
Duplicate comments:
In `@auditlog_clickhouse/data/auditlog_clickhouse_queue.xml`:
- Around line 13-26: Update the record with id
"action_configure_auditlog_clickhouse_cron" so its <field name="name"> uses a
clearer, descriptive label (e.g., "Configure ClickHouse flush schedule") instead
of "Edit export action"; locate the record by id and/or by res_model="ir.cron"
and res_id ref="auditlog_clickhouse.ir_cron_auditlog_clickhouse_enqueue_flush"
and replace the name text accordingly to reflect that this action configures the
ClickHouse enqueue/flush schedule.
In `@auditlog_clickhouse/models/auditlog_clickhouse_config.py`:
- Around line 342-392: The DDL in _get_clickhouse_ddl interpolates db_name
directly which can break on special/reserved names—replace direct interpolation
with a quoting helper (e.g., quote_identifier) and use it for the database and
table identifiers in both returned CREATE TABLE strings (replace occurrences
like {db_name}.auditlog_log and {db_name}.auditlog_log_line with quoted forms),
ensuring the helper emits ClickHouse-safe quoted identifiers (backticks or
double quotes) and is used consistently in that function.
In `@auditlog_clickhouse/models/auditlog_log_buffer.py`:
- Around line 300-315: The INSERT statements in _insert_rows_to_clickhouse
currently interpolate unquoted identifiers (config.database and table names)
which can break on reserved words or special chars; update the method to use the
shared identifier-quoting helper (e.g., _ch_ident) to produce safe identifiers
for the database and table (replace occurrences like
f"{config.database}.auditlog_log" and f"{config.database}.auditlog_log_line"
with the helper output), leaving the column lists (_CH_LOG_COLUMNS,
_CH_LINE_COLUMNS) and values (log_rows, line_rows) unchanged so the
client.execute calls use quoted identifiers.
In `@auditlog_clickhouse/models/auditlog_rule.py`:
- Around line 188-199: The base_log dict currently hard-codes "http_request_id"
and "http_session_id" to None, dropping any values in additional_log_values;
update the assignments in the base_log construction (the dict in
auditlog_rule.py that contains "model_id", "model_name", etc.) to use
additional_log_values.get("http_request_id") and
additional_log_values.get("http_session_id") (with a fallback to None) so any
provided request/session IDs are preserved when building the log payload.
In `@auditlog_clickhouse/readme/CONFIGURE.md`:
- Around line 30-31: Update the CONFIGURE.md text to remove the incorrect claim
that the module creates a database user: in the bullet that currently reads
"Optionally, click **Create Auditlog Tables** to create the tables and User in
the target database," delete "and User" so it only states the tables are
created; keep the rest of the wording and ensure the "Create Auditlog Tables"
phrase remains as-is to match the UI label.
In `@auditlog_clickhouse/readme/DESCRIPTION.md`:
- Line 1: Update the opening sentence in DESCRIPTION.md: replace the phrase
"This module implements buffered asynchronous transfers audit logs from
PostgreSQL to ClickHouse." with "This module implements buffered asynchronous
transfers of audit logs from PostgreSQL to ClickHouse." to add the missing
preposition "of" in the sentence.
In `@auditlog_clickhouse/tests/test_auditlog_clickhouse.py`:
- Around line 250-351: This file defines a concrete test class
TestAuditlogClickhouseConfig that duplicates other canonical config tests;
remove or convert this duplicate so tests aren't run twice: either delete this
TestAuditlogClickhouseConfig class entirely or make it non-concrete (e.g.,
rename to a different non-conflicting class or inherit/mark it abstract) and
ensure only the intended concrete subclass of AuditLogClickhouseCommon remains,
then run the grep from the review to verify a single
TestAuditlogClickhouseConfig definition across the codebase.
In `@requirements.txt`:
- Line 2: Replace the incorrect package name "clickhouse_driver" in
requirements.txt with the correct PyPI package name "clickhouse-driver" so pip
can resolve and install it; update the entry (the second line referencing
clickhouse_driver) to use the hyphenated form.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
auditlog_clickhouse/security/ir.model.access.csvis excluded by!**/*.csv
📒 Files selected for processing (22)
auditlog_clickhouse/README.rstauditlog_clickhouse/__init__.pyauditlog_clickhouse/__manifest__.pyauditlog_clickhouse/data/auditlog_clickhouse_queue.xmlauditlog_clickhouse/i18n/auditlog_clickhouse.potauditlog_clickhouse/models/__init__.pyauditlog_clickhouse/models/auditlog_clickhouse_config.pyauditlog_clickhouse/models/auditlog_log_buffer.pyauditlog_clickhouse/models/auditlog_rule.pyauditlog_clickhouse/models/clickhouse_client.pyauditlog_clickhouse/pyproject.tomlauditlog_clickhouse/readme/CONFIGURE.mdauditlog_clickhouse/readme/CONTEXT.mdauditlog_clickhouse/readme/DESCRIPTION.mdauditlog_clickhouse/readme/USAGE.mdauditlog_clickhouse/static/description/index.htmlauditlog_clickhouse/tests/__init__.pyauditlog_clickhouse/tests/common.pyauditlog_clickhouse/tests/test_auditlog_clickhouse.pyauditlog_clickhouse/tests/test_clickhouse_config.pyauditlog_clickhouse/views/auditlog_clickhouse_config_views.xmlrequirements.txt
tendil
force-pushed
the
18.0-t5246-auditlog_clickhouse-add_module
branch
2 times, most recently
from
df7fdac to
eed96ad
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
auditlog_clickhouse/models/auditlog_rule.py (1)
188-199:⚠️ Potential issue | 🟠 MajorPreserve request/session metadata from
additional_log_values.Lines 194–195 hard-code
http_request_idandhttp_session_idtoNone, discarding any values passed viaadditional_log_values. This breaks request/session traceability in ClickHouse logs.Proposed fix
base_log: dict[str, Any] = { "model_id": int(model_id), "model_name": model_rec.name, "model_model": model_rec.model, "user_id": int(uid), "method": method, - "http_request_id": None, - "http_session_id": None, + "http_request_id": additional_log_values.get("http_request_id"), + "http_session_id": additional_log_values.get("http_session_id"), "log_type": log_type, "create_date": now_iso, "create_uid": int(uid), }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@auditlog_clickhouse/models/auditlog_rule.py` around lines 188 - 199, The base_log dict currently hard-codes "http_request_id" and "http_session_id" to None and discards any values in additional_log_values; update the construction of base_log (the dict created in the function that builds audit log entries, referenced as base_log) to pull these two keys from additional_log_values when present (e.g. use additional_log_values.get("http_request_id") and .get("http_session_id") or merge additional_log_values into base_log after setting required defaults) so request/session metadata passed in is preserved in the ClickHouse log.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@auditlog_clickhouse/models/auditlog_rule.py`:
- Around line 245-269: The code currently browses the record and reads
record.display_name even during unlink where the record may be deleted; change
the logic to avoid browsing for unlink: only call model_rs.browse(res_id) and
use record.display_name when method is not "unlink" (and old_values is absent),
otherwise set name = old_values.get(res_id, EMPTY_DICT).get("display_name") if
old_values is present or None when it isn't (e.g., unlink_fast). Update the
block that builds log (around variables log_id, record, display_name,
old_values, method) to compute name from old_values first for unlink and fall
back to record.display_name only when safe.
In `@auditlog_clickhouse/tests/common.py`:
- Around line 43-55: create_config has a single line that exceeds the 88-char
limit (the return statement calling
cls.env["auditlog.clickhouse.config"].with_context(...).create(defaults)); split
that long expression into shorter statements to satisfy line-length checks:
assign the recordset to a short variable (e.g., cfg =
cls.env["auditlog.clickhouse.config"].with_context(tracking_disable=True)) and
then call cfg.create(defaults), or otherwise wrap the call across multiple lines
so no line exceeds 88 chars while keeping the same behavior.
- Around line 37-41: The method _cleanup_clickhouse_test_data has a line that
exceeds the 88-char limit; split the long statement that unlinks buffers into
shorter parts (e.g., assign cls.env["auditlog.log.buffer"].sudo().search([]) to
a local variable or call sudo()/search()/unlink() across multiple lines) so the
line length is <=88 while preserving the same calls and behavior; keep the
auditlog.clickhouse.config write line unchanged.
In `@auditlog_clickhouse/tests/test_auditlog_clickhouse.py`:
- Around line 62-76: The failing line in test_02_write_creates_lines is too
long; split the chained call into shorter statements: assign
self.env["res.groups"].with_context(tracking_disable=True) to a temporary
variable (e.g., groups_env), call groups_env.create({"name": "CH Group"}) and
then groups_env.write({"name": "CH Group v2"}) (or call group.with_context(...)
on a separate line) so lines stay under 88 chars while keeping the same behavior
in test_02_write_creates_lines.
---
Duplicate comments:
In `@auditlog_clickhouse/models/auditlog_rule.py`:
- Around line 188-199: The base_log dict currently hard-codes "http_request_id"
and "http_session_id" to None and discards any values in additional_log_values;
update the construction of base_log (the dict created in the function that
builds audit log entries, referenced as base_log) to pull these two keys from
additional_log_values when present (e.g. use
additional_log_values.get("http_request_id") and .get("http_session_id") or
merge additional_log_values into base_log after setting required defaults) so
request/session metadata passed in is preserved in the ClickHouse log.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
auditlog_clickhouse/models/auditlog_rule.pyauditlog_clickhouse/tests/common.pyauditlog_clickhouse/tests/test_auditlog_clickhouse.py
There was a problem hiding this comment.
♻️ Duplicate comments (2)
auditlog_clickhouse/models/auditlog_rule.py (2)
188-199:⚠️ Potential issue | 🟠 MajorPreserve request/session metadata from
additional_log_values.Lines 194-195 hardcode
http_request_idandhttp_session_idtoNone, discarding any values passed viaadditional_log_values. This breaks request/session traceability in ClickHouse logs.Proposed fix
base_log: dict[str, Any] = { "model_id": int(model_id), "model_name": model_rec.name, "model_model": model_rec.model, "user_id": int(uid), "method": method, - "http_request_id": None, - "http_session_id": None, + "http_request_id": additional_log_values.get("http_request_id"), + "http_session_id": additional_log_values.get("http_session_id"), "log_type": log_type, "create_date": now_iso, "create_uid": int(uid), }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@auditlog_clickhouse/models/auditlog_rule.py` around lines 188 - 199, The base_log dictionary currently hardcodes http_request_id and http_session_id to None, discarding any values provided in additional_log_values; update the construction of base_log (symbol: base_log in auditlog_rule.py) to pull http_request_id and http_session_id from additional_log_values (e.g. additional_log_values.get("http_request_id") and additional_log_values.get("http_session_id")) with a safe fallback to None so existing request/session metadata is preserved when present.
245-255:⚠️ Potential issue | 🟠 MajorAccessing
record.display_nameduring unlink is unsafe.During unlink operations,
model_rs.browse(res_id)at line 247 accesses a deleted or deleted-marked record. Callingrecord.display_nameat line 251 can raise errors or return stale/unexpected data.Proposed fix
for res_id in res_ids: log_id = str(uuid.uuid4()) - record = model_rs.browse(res_id) + if method == "unlink": + # Record may be deleted; use old_values if available + name = old_values.get(res_id, {}).get("display_name") + else: + record = model_rs.browse(res_id) + name = record.display_name log: _PayloadLog = { "id": log_id, - "name": record.display_name, + "name": name, "res_id": int(res_id), "res_ids": None, **base_log, }Note:
display_nameis typically a computed field and may not be inold_values. If preserving the display name is important for unlink logs, consider capturing it explicitly in the rule configuration.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@auditlog_clickhouse/models/auditlog_rule.py` around lines 245 - 255, The code currently browses records in the res_ids loop and directly reads record.display_name (model_rs.browse(res_id) then record.display_name) which can fail for unlinked/deleted records; change to safely fetch display_name via a read that ignores active_test (e.g., model_rs.with_context(active_test=False).read([res_id], ['display_name']) or model_rs.exists/read) and fall back to old_values/base_log (e.g., base_log.get("old_values", {}).get("display_name")) or None before building the _PayloadLog so that constructing the log dict (id, name, res_id, res_ids, **base_log) does not access deleted record attributes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@auditlog_clickhouse/models/auditlog_rule.py`:
- Around line 188-199: The base_log dictionary currently hardcodes
http_request_id and http_session_id to None, discarding any values provided in
additional_log_values; update the construction of base_log (symbol: base_log in
auditlog_rule.py) to pull http_request_id and http_session_id from
additional_log_values (e.g. additional_log_values.get("http_request_id") and
additional_log_values.get("http_session_id")) with a safe fallback to None so
existing request/session metadata is preserved when present.
- Around line 245-255: The code currently browses records in the res_ids loop
and directly reads record.display_name (model_rs.browse(res_id) then
record.display_name) which can fail for unlinked/deleted records; change to
safely fetch display_name via a read that ignores active_test (e.g.,
model_rs.with_context(active_test=False).read([res_id], ['display_name']) or
model_rs.exists/read) and fall back to old_values/base_log (e.g.,
base_log.get("old_values", {}).get("display_name")) or None before building the
_PayloadLog so that constructing the log dict (id, name, res_id, res_ids,
**base_log) does not access deleted record attributes.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
auditlog_clickhouse/models/auditlog_rule.pyauditlog_clickhouse/tests/common.pyauditlog_clickhouse/tests/test_auditlog_clickhouse.py
ivs-cetmix
requested changes
Feb 26, 2026
ivs-cetmix
left a comment
ivs-cetmix
left a comment
There was a problem hiding this comment.
Let's align docstrings so they all follow the required pattern:
Docstings must be complete. Must what the function does and why we need it. Document kwargs too if present. Each argument, exception and result must be documented. Always put data type for arguments and result. Don't put "Raises:" section in docstring if function doesn't raise any exception.
ivs-cetmix
requested changes
Mar 3, 2026
| :return: (excluded_fields_set, capture_record_flag) | ||
| :rtype: Tuple[Set[str], bool] | ||
| """ | ||
| cache: dict[tuple[int, tuple[int, ...]], tuple[set[str], bool]] = getattr( |
There was a problem hiding this comment.
Why no to use the @ormcache decorator for the function instead?
Author
There was a problem hiding this comment.
here @ormcache is not suitable because the result depends not only on model_id, but also on the current set of rules/exceptions
we need invalidation when rules change; auditlog already performs registry invalidation on rule changes so the cache on the pool is correctly and automatically cleared on reload
ormcache without an additional version/invalidation may return outdated settings
There was a problem hiding this comment.
What if the rules are changed but ID remains the same? Eg we modify only the 'capture_record'?
We need to cover this with tests.
Also we can simplify it:
if not hasattr(self.pool, "_auditlog_clickhouse_write_rule_cache"):
self.pool._auditlog_clickhouse_write_rule_cache = {}
cache = self.pool._auditlog_clickhouse_write_rule_cache| """ | ||
| return _json_sanitize(payload) | ||
|
|
||
| # flake8: noqa: C901 |
There was a problem hiding this comment.
Add an explanation why you need to disable this check.
Author
There was a problem hiding this comment.
this is one iteration and in the future I will divide this method into smaller related methods
Author
There was a problem hiding this comment.
this is one iteration and in the future I will divide this method into smaller related methods
also corrected
tendil
force-pushed
the
18.0-t5246-auditlog_clickhouse-add_module
branch
from
b6f0776 to
a1c6a5b
Compare
Aldeigja
changed the title
tendil
force-pushed
the
18.0-t5246-auditlog_clickhouse-add_module
branch
from
e1cbb0c to
fd90d57
Compare
ivs-cetmix
requested changes
Mar 5, 2026
| :return: (excluded_fields_set, capture_record_flag) | ||
| :rtype: Tuple[Set[str], bool] | ||
| """ | ||
| cache: dict[tuple[int, tuple[int, ...]], tuple[set[str], bool]] = getattr( |
There was a problem hiding this comment.
What if the rules are changed but ID remains the same? Eg we modify only the 'capture_record'?
We need to cover this with tests.
Also we can simplify it:
if not hasattr(self.pool, "_auditlog_clickhouse_write_rule_cache"):
self.pool._auditlog_clickhouse_write_rule_cache = {}
cache = self.pool._auditlog_clickhouse_write_rule_cache| f"{', '.join(self._CH_LOG_COLUMNS)}) VALUES", | ||
| log_rows, | ||
| ) | ||
| if line_rows: |
There was a problem hiding this comment.
What if log_rows are inserted correctly but then line_rows insertion fails?
In the next retry the log_rows will be reinserted again, so we will have those duplicated.
This case must be covered with test.
ivs-cetmix
requested changes
Mar 6, 2026
ivs-cetmix
left a comment
ivs-cetmix
left a comment
There was a problem hiding this comment.
Ensure that all the tables have all the fields you are using in queries.
Comment on lines
+59
to
+60
| "write_date", | ||
| "write_uid", |
There was a problem hiding this comment.
Are you sure these exist in the table?
Author
There was a problem hiding this comment.
thank you! I didn't fully transfer the necessary fields from read logic
i fixed it
tendil
force-pushed
the
18.0-t5246-auditlog_clickhouse-add_module
branch
from
5ae9a83 to
5101588
Compare
|
This PR has the |
tendil
force-pushed
the
18.0-t5246-auditlog_clickhouse-add_module
branch
2 times, most recently
from
f2e27d1 to
350ee69
Compare
Audit tables can grow without bounds on production databases, causing bloat and adding latency to audited operations because logs are written synchronously in the same transaction. Keeping audit data in PostgreSQL also leaves an immutability gap for privileged users. Introduce a dedicated module to buffer audit payloads and export them asynchronously to ClickHouse, allowing audit storage to scale without slowing down business transactions and keeping audit data effectively immutable in a write-only store. Task: 5246
Split ClickHouse existing-row checks into chunks to avoid oversized SELECT ... IN (...) queries during large imports. Previously, retry-safe deduplication could build a very large IN clause when checking already inserted log or log line ids. This caused ClickHouse to fail with "Max query size exceeded" and the queue job stopped processing buffered auditlog rows. The fix keeps deduplication logic but executes existence checks in smaller chunks, which preserves idempotent retries and prevents query size errors on bulk imports. Task: 5246
tendil
force-pushed
the
18.0-t5246-auditlog_clickhouse-add_module
branch
from
3b20173 to
3696841
Compare
Store auditlog HTTP session and request data in ClickHouse together with audit logs and log lines. Previously only auditlog_log and auditlog_log_line were exported. This made ClickHouse-backed logs incomplete for read scenarios that depend on http_session_id and http_request_id, especially after recreating the Odoo database. Also extend buffer processing and tests to handle HTTP-related rows and keep retry-safe deduplication for the additional tables. Task: 5246
tendil
force-pushed
the
18.0-t5246-auditlog_clickhouse-add_module
branch
from
3696841 to
b7137b6
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Task: 5246
Summary by CodeRabbit
New Features
Documentation
Tests
Chores