Skip to content

Conversation

@shaypal5
Copy link
Member

@shaypal5 shaypal5 commented Jul 3, 2025

No description provided.

@shaypal5 shaypal5 requested a review from Copilot July 3, 2025 10:03
@shaypal5 shaypal5 self-assigned this Jul 3, 2025

This comment was marked as outdated.

@shaypal5 shaypal5 requested a review from Copilot July 3, 2025 11:45

This comment was marked as outdated.

@shaypal5 shaypal5 requested a review from Copilot July 3, 2025 11:56

This comment was marked as outdated.

@shaypal5 shaypal5 marked this pull request as ready for review July 3, 2025 12:37
@shaypal5 shaypal5 requested review from Borda and Copilot July 3, 2025 12:37

This comment was marked as outdated.

@shaypal5 shaypal5 requested a review from Copilot July 3, 2025 12:48
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR introduces a SQL-based caching core for Cachier, enabling use of SQLAlchemy backends alongside existing pickle, memory, and MongoDB options. Key changes include

  • Implementation of a new _SQLCore class in src/cachier/cores/sql.py, with full CRUD and concurrency handling
  • Comprehensive test suite in tests/test_sql_core.py plus CI and requirements updates to run SQL-backed tests
  • Updates to cachier decorator, documentation in README.rst, and CI configurations to integrate the SQL backend

Reviewed Changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
tests/test_sql_core.py Add end-to-end and edge-case tests for SQL core
src/cachier/cores/sql.py Implement the _SQLCore class using SQLAlchemy
src/cachier/core.py Wire in backend="sql" and sql_engine parameter
tests/sql_requirements.txt Add SQLAlchemy and psycopg2 dependencies
README.rst Document SQL core usage, schema, and limitations
.github/workflows/ci-test.yml Extend CI matrix to run sql-marked tests
pyproject.toml Register sql pytest marker and lint exceptions
Comments suppressed due to low confidence (1)

tests/test_sql_core.py:64

  • Consider adding a test for the next_time=True case to verify that stale entries are returned immediately when the next_time flag is enabled.
        next_time=False,

@shaypal5
Copy link
Member Author

shaypal5 commented Jul 3, 2025

Ok, this is ready for a review.
Wanna take a look? :)
@Borda

shaypal5 and others added 3 commits July 3, 2025 22:27
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@shaypal5 shaypal5 merged commit 410543c into master Jul 3, 2025
31 of 32 checks passed
@shaypal5 shaypal5 deleted the sql-core-2 branch July 3, 2025 19:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants