Skip to content

Modernize Records: Add Type Hints, Async Support, Enhanced Context Managers & CI/CD #229

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Modernize Records: Add Type Hints, Async Support, Enhanced Context Managers & CI/CD #229

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
47fa177
first commit
Yash11778 Oct 18, 2025
505e8b3
Fix GitHub Actions workflow issues
Yash11778 Oct 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
96 changes: 96 additions & 0 deletions .github/workflows/basic-test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
name: Basic Tests

on:
push:
branches: [ master, main, dev ]
pull_request:
branches: [ master, main ]

jobs:
test:
runs-on: ${{ matrix.os }}
strategy:
fail-fast: false
matrix:
os: [ubuntu-latest, windows-latest, macos-latest]
python-version: ['3.8', '3.9', '3.10', '3.11']
Copy link

@coderabbitai coderabbitai bot Oct 18, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Python 3.8 incompatibility with latest pytest.

Same issue: pytest 8.4.2 dropped Python 3.8 support.

-        python-version: ['3.8', '3.9', '3.10', '3.11']
+        python-version: ['3.9', '3.10', '3.11', '3.12']

Based on learnings.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
python-version: ['3.8', '3.9', '3.10', '3.11']
python-version: ['3.9', '3.10', '3.11', '3.12']
🤖 Prompt for AI Agents 
.github/workflows/basic-test.yml around line 16: the matrix includes
python-version '3.8' but recent pytest releases (>=8.4.2) no longer support
Python 3.8; remove '3.8' from the python-version list in the matrix to avoid
running tests under an unsupported interpreter (alternatively, pin pytest to a
3.8-compatible version like <=8.3.x in your test dependencies if you must keep
3.8).


steps:
- uses: actions/checkout@v4

- name: Set up Python ${{ matrix.python-version }}
uses: actions/setup-python@v4
with:
python-version: ${{ matrix.python-version }}

- name: Install dependencies
run: |
python -m pip install --upgrade pip
python -m pip install -e .

- name: Test basic import
run: |
python -c "import records; print('✓ Records import successful')"

- name: Test basic functionality
run: |
python -c "
import records
db = records.Database('sqlite:///:memory:')
db.query('CREATE TABLE test (id INTEGER)')
db.query('INSERT INTO test VALUES (1)')
result = db.query('SELECT * FROM test')
assert len(list(result)) == 1
db.close()
print('✓ Basic functionality works')
"

- name: Run enhancement tests
run: |
python test_enhancements_simple.py

- name: Test context managers
run: |
python test_context_manager.py

integration:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4

- name: Set up Python
uses: actions/setup-python@v4
with:
python-version: '3.10'

- name: Install dependencies
run: |
python -m pip install --upgrade pip
python -m pip install -e .

- name: Run full integration test
run: |
python final_integration_test.py

lint:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4

- name: Set up Python
uses: actions/setup-python@v4
with:
python-version: '3.10'

- name: Install linting tools
run: |
python -m pip install --upgrade pip
python -m pip install flake8 mypy

- name: Lint with flake8 (non-blocking)
run: |
flake8 records.py --count --exit-zero --max-complexity=10 --max-line-length=127 --statistics || true

- name: Type check with mypy (non-blocking)
run: |
mypy records.py --ignore-missing-imports || true
Comment on lines +90 to +96
Copy link

@coderabbitai coderabbitai bot Oct 18, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Non-blocking lint steps defeat their purpose.

The lint and mypy steps use || true to prevent failures, making them effectively informational only. This allows code quality issues to pass unchecked.

Remove the non-blocking behavior to enforce code quality:

     - name: Lint with flake8 (non-blocking)
       run: |
-        flake8 records.py --count --exit-zero --max-complexity=10 --max-line-length=127 --statistics || true
+        flake8 records.py --count --max-complexity=10 --max-line-length=127 --statistics
         
     - name: Type check with mypy (non-blocking)
       run: |
-        mypy records.py --ignore-missing-imports || true
+        mypy records.py --ignore-missing-imports

If you want non-blocking during initial rollout, use continue-on-error: true at the step level instead, which still records failures in the workflow status.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- name: Lint with flake8 (non-blocking)
run: |
flake8 records.py --count --exit-zero --max-complexity=10 --max-line-length=127 --statistics || true
- name: Type check with mypy (non-blocking)
run: |
mypy records.py --ignore-missing-imports || true
- name: Lint with flake8 (non-blocking)
run: |
flake8 records.py --count --max-complexity=10 --max-line-length=127 --statistics
- name: Type check with mypy (non-blocking)
run: |
mypy records.py --ignore-missing-imports
🤖 Prompt for AI Agents 
.github/workflows/basic-test.yml lines 90-96: the lint and mypy steps append "||
true" which masks failures and makes them non-blocking; remove the "|| true"
from each run line so the commands fail the job on errors, or alternatively
replace the inline suppression by setting "continue-on-error: true" at the step
level if you want failures recorded but not to stop the workflow; keep the
existing flags for flake8 and mypy but ensure the step exits with non-zero on
errors by eliminating the shell-level suppression.

61 changes: 61 additions & 0 deletions .github/workflows/release.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
name: Release

on:
release:
types: [published]

jobs:
test:
runs-on: ubuntu-latest
strategy:
matrix:
python-version: ['3.8', '3.9', '3.10', '3.11', '3.12']
Copy link

@coderabbitai coderabbitai bot Oct 18, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Python 3.8 incompatibility with latest pytest.

Python 3.8 support was dropped in pytest 8.4.2, but pyproject.toml specifies pytest>=6.0 without an upper bound, meaning the latest pytest will be installed by default. Additionally, Python 3.12 is in the test matrix but missing from the package classifiers in pyproject.toml (lines 22-27).

Update the Python version matrix to align with supported versions and add an upper bound for pytest if continuing to support Python 3.8:

-        python-version: ['3.8', '3.9', '3.10', '3.11', '3.12']
+        python-version: ['3.9', '3.10', '3.11', '3.12']

Or, if Python 3.8 support is required, pin pytest in pyproject.toml:

test = [
    "pytest>=6.0,<8.4",
    "pytest-cov",
]

Based on learnings.


steps:
- uses: actions/checkout@v4

- name: Set up Python ${{ matrix.python-version }}
uses: actions/setup-python@v4
with:
python-version: ${{ matrix.python-version }}

- name: Install dependencies
run: |
python -m pip install --upgrade pip
python -m pip install -e ".[test]"

- name: Test with pytest
run: |
pytest -v

build-and-publish:
needs: test
runs-on: ubuntu-latest

steps:
- uses: actions/checkout@v4

- name: Set up Python
uses: actions/setup-python@v4
with:
python-version: '3.10'

- name: Install build dependencies
run: |
python -m pip install --upgrade pip
python -m pip install build twine

- name: Build package
run: |
python -m build

- name: Check distribution
run: |
python -m twine check dist/*

- name: Publish to PyPI
env:
TWINE_USERNAME: __token__
TWINE_PASSWORD: ${{ secrets.PYPI_API_TOKEN }}
run: |
python -m twine upload dist/*
73 changes: 73 additions & 0 deletions .github/workflows/test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
name: Tests

on:
push:
branches: [ master, main, dev ]
pull_request:
branches: [ master, main ]

jobs:
test:
runs-on: ${{ matrix.os }}
strategy:
fail-fast: false
matrix:
os: [ubuntu-latest, windows-latest, macos-latest]
python-version: ['3.8', '3.9', '3.10', '3.11']
Copy link

@coderabbitai coderabbitai bot Oct 18, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Python 3.8 incompatibility and workflow inconsistency.

Same issue as release.yml: pytest 8.4.2 dropped Python 3.8 support. Additionally, this workflow tests Python 3.8-3.11 while release.yml tests 3.8-3.12, creating inconsistent coverage.

Standardize Python versions across all workflows:

-        python-version: ['3.8', '3.9', '3.10', '3.11']
+        python-version: ['3.9', '3.10', '3.11', '3.12']

Based on learnings.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
python-version: ['3.8', '3.9', '3.10', '3.11']
python-version: ['3.9', '3.10', '3.11', '3.12']
🤖 Prompt for AI Agents 
.github/workflows/test.yml around line 16: the matrix lists python-version:
['3.8','3.9','3.10','3.11'], which is incompatible with pytest 8.4.2 (dropped
3.8) and mismatches release.yml; update the matrix to remove Python 3.8 and
standardize versions to match release.yml (e.g., ['3.9','3.10','3.11','3.12'])
so both workflows test the same supported Python versions.


steps:
- uses: actions/checkout@v4

- name: Set up Python ${{ matrix.python-version }}
uses: actions/setup-python@v4
with:
python-version: ${{ matrix.python-version }}

- name: Install dependencies
run: |
python -m pip install --upgrade pip
python -m pip install -e .

- name: Test basic import
run: |
python -c "import records; print('✓ Records import successful')"

- name: Test basic functionality
run: |
python -c "
import records
db = records.Database('sqlite:///:memory:')
db.query('CREATE TABLE test (id INTEGER)')
db.query('INSERT INTO test VALUES (1)')
result = db.query('SELECT * FROM test')
assert len(list(result)) == 1
db.close()
print('✓ Basic functionality works')
"

- name: Run enhancement tests
run: |
python test_enhancements_simple.py

integration:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4

- name: Set up Python
uses: actions/setup-python@v4
with:
python-version: '3.10'

- name: Install dependencies
run: |
python -m pip install --upgrade pip
python -m pip install -e .

- name: Test context managers
run: |
python test_context_manager.py

- name: Run full integration test
run: |
python final_integration_test.py
Loading