Skip to content

PR 1684 and 1695 review#1700

Closed
josecelano wants to merge 9 commits into
torrust:developfrom
josecelano:pr-1684-review
Closed

PR 1684 and 1695 review#1700
josecelano wants to merge 9 commits into
torrust:developfrom
josecelano:pr-1684-review

Conversation

@josecelano
Copy link
Copy Markdown
Member

@josecelano josecelano commented Apr 21, 2026

Do not merge this!

It's just for the record. It's my review for the PRs:

The file docs/pr-reviews/pr-1684-postgresql-driver.md contains a complete review.

This comment contains a proposal to do a rework organizing the changes into many sub-issues that implement the new driver progresively.

josecelano added a commit to josecelano/torrust-tracker that referenced this pull request May 1, 2026
Close EPIC torrust#1525 by moving the EPIC issue document and the final PostgreSQL sub-issue document to docs/issues/closed after finishing the incremental implementation plan.

History:
- The EPIC started months ago when we identified that adding PostgreSQL support was not straightforward with the previous persistence crate and required migration to sqlx.
- DamnCrab opened PR torrust#1684, followed by PR torrust#1695 after review feedback.
- Additional reviewer guidance was provided through PR torrust#1700 to help address change requests.
- The final implementation was completed through the incremental plan documented in issue comment #4294475557 on issue torrust#1525.
- The tracker now supports PostgreSQL.

Ideas initially introduced in DamnCrab's PRs and retained in the final implementation:
- Matrix DB compatibility script to validate tracker compatibility across database versions.
- End-to-end tracker tests using a real BitTorrent client (containerized qBittorrent).
- Basic database benchmarking to compare persistence performance before/after migration to sqlx and across database engines.

Thanks to DamnCrab for valuable contributions and ideas that improved the final result.

Co-authored-by: DamnCrab <42539593+DamnCrab@users.noreply.github.com>
@josecelano
Copy link
Copy Markdown
Member Author

Deprecated. See #1725

@josecelano josecelano closed this May 1, 2026
josecelano added a commit that referenced this pull request May 1, 2026
9101e76 docs(issues): close epic 1525 persistence overhaul (Jose Celano)

Pull request description:

  ## Summary
  This PR closes the EPIC by moving completed issue documents into the closed issues folder:

  - `docs/issues/1525-overhaul-persistence.md` -> `docs/issues/closed/1525-overhaul-persistence.md`
  - `docs/issues/1723-1525-08-add-postgresql-driver.md` -> `docs/issues/closed/1723-1525-08-add-postgresql-driver.md`

  ## EPIC History
  The EPIC was created months ago when we identified that adding PostgreSQL support was not straightforward with the previous persistence crate. The work required migrating persistence to `sqlx`.

  A community contributor, [DamnCrab](https://github.com/DamnCrab), opened PR [#1684](#1684). After review feedback, they opened a follow-up PR [#1695](#1695). To help address the requested changes, additional reviewer guidance and code was provided in PR [#1700](#1700).

  The final implementation was then completed through the incremental plan documented in issue comment:
  - #1525 (comment)

  That plan is now complete, and Torrust Tracker supports PostgreSQL.

  ## Acknowledgement
  Thanks to [DamnCrab](https://github.com/DamnCrab) for valuable contributions and ideas retained in the final implementation:

  1. A DB compatibility matrix script to validate tracker compatibility across database versions.
  2. End-to-end tests using a real BitTorrent client (containerized qBittorrent).
  3. Basic database benchmarking to compare persistence performance before/after the `sqlx` migration and across database engines.

  Closes #1525

ACKs for top commit:
  josecelano:
    ACK 9101e76

Tree-SHA512: 50bde2ba45f82a1d2779572593138e1d61a6e84b180ed61babe5c86032c20c925e1abe3b4da096fe6783dd467cee554151d6fed0090b6d59d29880d1182f1a8c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants