Code consistency#358
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @PGijsbers, your pull request is larger than the review limit of 150000 diff characters
for more information, see https://pre-commit.ci
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
💤 Files with no reviewable changes (1)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughThe pull request migrates database access from Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #358 +/- ##
===========================================
+ Coverage 77.60% 94.62% +17.01%
===========================================
Files 77 77
Lines 3774 3815 +41
Branches 247 248 +1
===========================================
+ Hits 2929 3610 +681
+ Misses 759 137 -622
+ Partials 86 68 -18 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/routers/datasets.py (1)
137-156: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winInvalid Python 3 except clause
except DatasetNotFoundError, DatasetNoAccessError:is syntax-invalid and will stopsrc/routers/datasets.pyfrom loading. Useexcept (DatasetNotFoundError, DatasetNoAccessError):instead.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/routers/datasets.py` around lines 137 - 156, Fix the invalid exception syntax in the untag_dataset function by changing the except clause to catch DatasetNotFoundError and DatasetNoAccessError as a tuple: except (DatasetNotFoundError, DatasetNoAccessError):.
🧹 Nitpick comments (1)
src/database/tasks.py (1)
21-149: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftInconsistent DB-handle parameter naming (
expdbvssession) undermines this PR's consistency goal.Within this same file, most functions take
expdb: AsyncSession(Lines 21, 35, 47, 61, 82, 104-106, 121, 135-137) whileget_tagsandtagtakesession: AsyncSession(Lines 152, 162). This split isn't tied to raw-SQL vs ORM usage either —src/database/setups.py'sget()(also raw SQL viasession.execute) usessession, whilesrc/database/runs.py/flows.py'sget()useexpdb. Since this PR's explicit goal is naming/usage consistency, consider standardizing on one parameter name across all database helper modules.Also applies to: 152-176
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/database/tasks.py` around lines 21 - 149, Standardize the database-handle parameter name across the helper modules to match the consistency goal. Rename the inconsistent `expdb`/`session` parameters in `get`, `get_task_types`, `get_task_type`, `get_task_type_name`, `get_task_evaluation_measure`, `get_input_for_task_type`, `get_input_for_task`, `get_task_type_inout_with_template`, `get_tags`, and `tag`, updating all internal references and call sites consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/development/tests.md`:
- Around line 95-106: Update the AsyncConnection.execute calls in
test_python_and_php to use the parameters= keyword instead of params= for both
the dataset INSERT and DELETE statements.
In `@src/routers/tasks.py`:
- Around line 472-474: In the loop building filled_templates, replace the unused
name variable in the input_templates iteration with an underscore while
retaining template for fill_template.
In `@tests/routers/users_delete_test.py`:
- Around line 55-61: Update the stale comment near the user setup in the test to
replace the old user_test fixture reference with userdb_session, and accurately
state that rollback is performed by the dependent userdb_connection fixture.
Keep the comment consistent with the current fixture names and behavior.
---
Outside diff comments:
In `@src/routers/datasets.py`:
- Around line 137-156: Fix the invalid exception syntax in the untag_dataset
function by changing the except clause to catch DatasetNotFoundError and
DatasetNoAccessError as a tuple: except (DatasetNotFoundError,
DatasetNoAccessError):.
---
Nitpick comments:
In `@src/database/tasks.py`:
- Around line 21-149: Standardize the database-handle parameter name across the
helper modules to match the consistency goal. Rename the inconsistent
`expdb`/`session` parameters in `get`, `get_task_types`, `get_task_type`,
`get_task_type_name`, `get_task_evaluation_measure`, `get_input_for_task_type`,
`get_input_for_task`, `get_task_type_inout_with_template`, `get_tags`, and
`tag`, updating all internal references and call sites consistently.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 4a86116c-015e-494e-8247-fbe17668d0cd
📒 Files selected for processing (57)
docs/development/tests.mdsrc/core/access.pysrc/database/datasets.pysrc/database/evaluations.pysrc/database/flows.pysrc/database/qualities.pysrc/database/runs.pysrc/database/setups.pysrc/database/studies.pysrc/database/tasks.pysrc/database/users.pysrc/routers/datasets.pysrc/routers/dependencies.pysrc/routers/estimation_procedure.pysrc/routers/evaluations.pysrc/routers/flows.pysrc/routers/qualities.pysrc/routers/runs.pysrc/routers/schemas/__init__.pysrc/routers/schemas/core.pysrc/routers/schemas/datasets.pysrc/routers/schemas/flows.pysrc/routers/schemas/runs.pysrc/routers/schemas/setups.pysrc/routers/schemas/study.pysrc/routers/schemas/tasks.pysrc/routers/setups.pysrc/routers/study.pysrc/routers/tasks.pysrc/routers/tasktype.pysrc/routers/users.pytests/conftest.pytests/database/flows_test.pytests/database/runs_test.pytests/dependencies/fetch_user_test.pytests/routers/dataset_tag_test.pytests/routers/dataset_untag_test.pytests/routers/datasets_features_test.pytests/routers/datasets_get_test.pytests/routers/datasets_list_datasets_test.pytests/routers/datasets_qualities_test.pytests/routers/datasets_status_test.pytests/routers/flows_exists_test.pytests/routers/flows_get_test.pytests/routers/qualities_list_test.pytests/routers/runs_get_test.pytests/routers/runs_trace_test.pytests/routers/setups_get_test.pytests/routers/setups_tag_test.pytests/routers/setups_untag_test.pytests/routers/study_attach_test.pytests/routers/tag_test_helper.pytests/routers/task_get_test.pytests/routers/task_list_test.pytests/routers/task_tag_test.pytests/routers/task_type_get_test.pytests/routers/users_delete_test.py
1 participant
Several changes to make the code base more consistent in naming, typing, and use of connection vs session.