-
Notifications
You must be signed in to change notification settings - Fork 0
New folder project structure #82
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
WalkthroughRestructures the project by organizing writer modules into Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
|
Trivy has completed a full security repository scan ✅ You can find the analysis results for this PR branch on this overview.
|
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
🧹 Nitpick comments (1)
src/writers/writer_postgres.py (1)
30-30: Remove unusednoqadirective.The
# noqa: F401directive is unnecessary sincepsycopg2is used later in the code (line 37 onwards).Apply this diff:
- import psycopg2 # noqa: F401 + import psycopg2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
src/event_gate_lambda.py(1 hunks)src/utils/__init__.py(1 hunks)src/utils/conf_path.py(3 hunks)src/utils/trace_logging.py(1 hunks)src/writers/__init__.py(1 hunks)src/writers/writer_eventbridge.py(1 hunks)src/writers/writer_kafka.py(1 hunks)src/writers/writer_postgres.py(1 hunks)tests/__init__.py(1 hunks)tests/utils/__init__.py(1 hunks)tests/utils/test_conf_path.py(5 hunks)tests/utils/test_conf_validation.py(1 hunks)tests/utils/test_safe_serialization.py(2 hunks)tests/utils/test_trace_logging.py(1 hunks)tests/writers/__init__.py(1 hunks)tests/writers/test_writer_eventbridge.py(1 hunks)tests/writers/test_writer_kafka.py(1 hunks)tests/writers/test_writer_postgres.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
src/utils/trace_logging.py (1)
src/utils/safe_serialization.py (1)
safe_serialize_for_log(47-84)
src/writers/writer_kafka.py (3)
src/utils/trace_logging.py (1)
log_payload_at_trace(29-48)tests/utils/test_trace_logging.py (2)
produce(26-29)flush(31-32)tests/writers/test_writer_kafka.py (7)
produce(10-13)produce(20-22)produce(92-93)flush(15-16)flush(31-38)flush(47-49)flush(58-60)
src/writers/writer_eventbridge.py (1)
src/utils/trace_logging.py (1)
log_payload_at_trace(29-48)
src/writers/writer_postgres.py (3)
src/utils/trace_logging.py (1)
log_payload_at_trace(29-48)tests/writers/test_writer_postgres.py (7)
client(243-244)cursor(174-175)execute(34-35)execute(159-160)connect(191-192)connect(227-228)commit(177-178)tests/utils/test_trace_logging.py (4)
cursor(59-60)execute(49-50)connect(72-73)commit(62-63)
tests/utils/test_safe_serialization.py (1)
src/utils/safe_serialization.py (1)
safe_serialize_for_log(47-84)
🪛 GitHub Check: Trivy
src/writers/writer_kafka.py
[failure] 73-73: Plaintext password literal
Artifact: src/writers/writer_kafka.py
Type:
Secret Plaintext password literal
Severity: HIGH
Match: "ssl.key.********************************************
src/writers/writer_postgres.py
[failure] 67-67: Plaintext secret literal
Artifact: src/writers/writer_postgres.py
Type:
Secret Plaintext secret literal
Severity: HIGH
Match: ********************************************************
[failure] 68-68: Plaintext secret literal
Artifact: src/writers/writer_postgres.py
Type:
Secret Plaintext secret literal
Severity: HIGH
Match: ************************************************************
[failure] 70-71: Plaintext secret literal
Artifact: src/writers/writer_postgres.py
Type:
Secret Plaintext secret literal
Severity: HIGH
Match: if secret_name and **************
[failure] 72-72: Plaintext secret literal
Artifact: src/writers/writer_postgres.py
Type:
Secret Plaintext secret literal
Severity: HIGH
Match: ************************************************************************************
[failure] 282-282: Plaintext password literal
Artifact: src/writers/writer_postgres.py
Type:
Secret Plaintext password literal
Severity: HIGH
Match: ******************************
🪛 Ruff (0.14.4)
src/writers/writer_kafka.py
58-58: Avoid specifying long messages outside the exception class
(TRY003)
109-109: Unused lambda argument: msg
(ARG005)
src/writers/writer_postgres.py
30-30: Unused noqa directive (non-enabled: F401)
Remove unused noqa directive
(RUF100)
90-120: Possible SQL injection vector through string-based query construction
(S608)
149-171: Possible SQL injection vector through string-based query construction
(S608)
186-206: Possible SQL injection vector through string-based query construction
(S608)
229-247: Possible SQL injection vector through string-based query construction
(S608)
299-299: Use explicit conversion flag
Replace with conversion flag
(RUF010)
🔇 Additional comments (10)
tests/writers/test_writer_eventbridge.py (1)
4-4: Imports aligned with new package layout.Switching to
src.writers.writer_eventbridgekeeps the tests in step with the reorganized module structure. Looks good.tests/utils/test_trace_logging.py (1)
4-7: LGTM!The import path updates correctly reflect the new package structure with writers under
src.writers/and utilities undersrc.utils/.tests/writers/test_writer_postgres.py (1)
22-22: LGTM!The import path correctly reflects the new package structure.
src/writers/writer_kafka.py (1)
29-29: LGTM!The import path correctly reflects the new utility module location under
src.utils/.src/utils/conf_path.py (1)
37-39: LGTM!The project root calculation correctly reflects the new file location at
src/utils/conf_path.py(two levels up from root instead of one). The intermediate variableparent_utils_dirimproves clarity.tests/utils/test_conf_validation.py (1)
22-22: LGTM!The CONF_DIR path calculation correctly navigates from the new test location (
tests/utils/) to the repository rootconf/directory.src/writers/writer_postgres.py (1)
34-34: LGTM!The import path correctly reflects the new utility module location under
src.utils/.src/event_gate_lambda.py (1)
34-39: LGTM!The updated imports correctly reflect the new package structure, and the internal aliases maintain compatibility within the module. The removal of conditional import fallbacks simplifies the code.
tests/utils/test_conf_path.py (2)
7-7: LGTM!The import path correctly reflects the new module location.
25-26: LGTM!The test assertions correctly compute the repository root conf path by going three levels up from the module file location (
src/utils/conf_path.py), which aligns with the updatedproject_rootcalculation in the source module.Also applies to: 83-84, 125-126, 156-157
oto-macenauer-absa
left a 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.
lgtm, the aquasec warning are nonsense/false positives
Overview
The project is getting more complex, so there is need of having a new folder clear project structure.
Release Notes:
Closes #77
Summary by CodeRabbit
Release Notes
Refactor
Tests