Skip to content

Conversation

mredenti
Copy link

@mredenti mredenti commented Oct 6, 2025

This PR introduces SQLAlchemy Core to abstract database interactions in Reframe's storage backend, adding support for PostgreSQL alongside SQLite. It serves as a proof-of-concept to gather feedback on this approach before adding comprehensive tests and documentation. I have manually tested PostgreSQL with psycopg2 for schema creation, storage, and CLI output. Full PostgreSQL tests in test_storage.py will follow approval.

Motivation

Supporting multiple SQL dialects (SQLite, PostgreSQL, MySQL) is complex due to differences in:

  • Connection APIs (e.g., PostgreSQL needs drivers like psycopg2 or pg8000)
  • Schema syntax (e.g., SQLite's AUTOINCREMENT vs. PostgreSQL's SERIAL)
  • Transaction behaviors and foreign key handling
  • Risk of UUID collisions in concurrent sessions with application-generated IDs

SQLAlchemy Core unifies these interactions, improving maintainability and extensibility.

Next Steps (pending feedback)

  • Add PostgreSQL tests to test_storage.py
  • Update documentation for PostgreSQL configuration
  • Implement database-generated IDs for session UUIDs
  • Support report caching for retry on storage failures

Note Users can replace psycopg2 with their preferred PostgreSQL driver (e.g., pg8000) by updating requirements.txt and postgresql_driver in the system configuration. Suggestions on test cases or PostgreSQL edge cases are welcome.

…e-only behaviour and schema layout.

- Introduce _ConnectionStrategy class and _SqliteConnector sub-classe to
  encapsulate URL building, engine creation, and per-dialect kwargs.
- Build schema with SQLAlchemy Core (MetaData, Table, Column), preserving table/column names and the index_testcases_time index.
- Convert INSERT, SELECT, DELETE queries to Core expressions.
- Enable SQLite foreign keys via PRAGMA foreign_keys=ON on each connection (matches previous cascade semantics).
- Replace raw sqlite3 connection/locking with SQLAlchemy transactional contexts (engine.begin())
- Drop legacy file lock code and direct sqlite3.connect() calls.
- Remove uuids_sql raw string building in remove paths; use SQLAlchemy's in_() instead.
- Introduce _PostgresConnector and wire backend='postgresql' in
  StorageBackend.create().
- Read connection parameters from site config: storage/0/postgresql_driver, postgresql_host, postgresql_port,
    postgresql_db, postgresql_conn_timeout and credentials from env: RFM_POSTGRES_USER / RFM_POSTGRES_PASSWORD.
- Pass connect_timeout via DBAPI connect args.
- Extend config schema (schemas/config.json) with the new Postgres options
  and add sensible defaults (driver=psycopg2, conn_timeout=60).
- Keep SQLite as-is; no behavior change for existing users.
- Add psycopg2-binary==2.9.8 to requirements.txt to provide the PG driver.
- Add ConnectionStrategy.json_column_type property (default Text, overridden
  to JSONB for Postgres).
- Use this type in schema so Postgres stores sessions.json_blob as JSONB.
- Normalize JSONB values in _decode_sessions() by serializing non-strings to
  JSON strings for consistent downstream parsing.
- CLI: display backend info depending on storage backend
  (SQLite file path vs PostgreSQL host:port/db).
- write report as encoded JSON if postgresql
Copy link

codecov bot commented Oct 6, 2025

Codecov Report

❌ Patch coverage is 78.33333% with 26 lines in your changes missing coverage. Please review.
✅ Project coverage is 69.17%. Comparing base (ce5a2e3) to head (7aa7d78).

Files with missing lines Patch % Lines
reframe/frontend/reporting/storage.py 81.08% 21 Missing ⚠️
reframe/frontend/cli.py 44.44% 5 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (ce5a2e3) and HEAD (7aa7d78). Click for more details.

HEAD has 13 uploads less than BASE
Flag BASE (ce5a2e3) HEAD (7aa7d78)
23 10
Additional details and impacted files
@@             Coverage Diff              @@
##           develop    #3567       +/-   ##
============================================
- Coverage    91.22%   69.17%   -22.05%     
============================================
  Files           62       60        -2     
  Lines        13525    13482       -43     
============================================
- Hits         12338     9326     -3012     
- Misses        1187     4156     +2969     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@mredenti mredenti changed the title Feat/storage sqlalchemy [feat] Addind support for PostgreSQL storage via SQLAlchemy Oct 7, 2025
@mredenti mredenti changed the title [feat] Addind support for PostgreSQL storage via SQLAlchemy [feat] Adding support for PostgreSQL storage via SQLAlchemy Oct 7, 2025
@vkarak vkarak self-requested a review October 7, 2025 22:16
@vkarak vkarak added request for enhancement reporting Issues related to reporting and processing the test results prio: normal labels Oct 7, 2025
@vkarak vkarak moved this to Todo in ReFrame Backlog Oct 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
prio: normal reporting Issues related to reporting and processing the test results request for enhancement
Projects
Status: Todo
Development

Successfully merging this pull request may close these issues.

2 participants