Skip to content
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.

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

Support sqlite/postgresql backend #498

Open
Glandos opened this issue Mar 18, 2020 · 18 comments · May be fixed by #1520
Open

Support sqlite/postgresql backend #498

Glandos opened this issue Mar 18, 2020 · 18 comments · May be fixed by #1520
Labels
13 Estimate - xxl - Complex, large effort, well defined (these should be broken down further) enhancement New feature or request good first bug Low priority, but valuable contributions

Comments

@Glandos
Copy link

Glandos commented Mar 18, 2020

In the Python version of syncserver, SQL was handled by SQLAlchemy, and thus every major SQL backends were available. I personally use sqlite because I'm nearly a single user.

As a I discover this Rust rewrite (:clap:), I see that only MySQL and Spanner are currently supported. This is a bit unfortunate, since it lefts all sqlite and postgresql existing users without alternative. I withdraw MySQL very long time ago, to have only one DB engine to manage.

I also understand that in can be a non-goal to support other architecture than Mozilla. I just wanted to ask: what are the plans for backends?

@jrconlin jrconlin added 8 Estimate - xl - Moderately complex, medium effort, some uncertainty. enhancement New feature or request labels Mar 18, 2020
@jrconlin
Copy link
Member

I don't think any of us on the 3 person team are opposed to supporting other databases, it's just that we've got a fair bit on our plate right now and are focused on the spanner side of things at the moment.

The good news is that the SQL calls we do are very simple. We do need to make sure that the MySQL calls are compatible with what we're doing with Spanner, but that should be more of a function check.

Needless to say, we always welcome folks helping us out so we can deliver this feature sooner.

@fzzzy
Copy link
Contributor

fzzzy commented Mar 21, 2020

@Glandos We use diesel for the mysql backend. Diesel supports mysql, postgres, and sqlite. So perhaps with a little tinkering the existing mysql diesel code could be repurposed. I'll mark it as a good first bug.

@fzzzy fzzzy added the good first bug Low priority, but valuable contributions label Mar 21, 2020
@pjenvey pjenvey added the self-hosting-phase1 Related to self hosting support (Phase I, Mysql Support) label May 18, 2020
@pjenvey pjenvey added 13 Estimate - xxl - Complex, large effort, well defined (these should be broken down further) self-hosting-phase2 Related to self hosting support (Phase II) and removed 8 Estimate - xl - Moderately complex, medium effort, some uncertainty. self-hosting-phase1 Related to self hosting support (Phase I, Mysql Support) labels May 28, 2020
@tublitzed tublitzed removed the self-hosting-phase2 Related to self hosting support (Phase II) label Aug 18, 2020
@mqus mqus mentioned this issue Nov 30, 2020
2 tasks
@BernardIgiri BernardIgiri removed their assignment Oct 8, 2021
@somini
Copy link

somini commented Apr 27, 2022

#935 implements this, any reason why this wasn't merged?

@jrconlin
Copy link
Member

It's currently failing tests, and unfortunately, we don't have the resources to continue work on this internally at this time.

Others are more than welcome to continue work and resolve the outstanding issues. (Please be sure to sign your commits, otherwise our CI will not allow the code to be merged.)

@somini
Copy link

somini commented Apr 27, 2022

Thanks for the candor.

I tried rebasing and running the tests, but this is above my paygrade (at least for after-hours hacking). Maybe @mqus can continue its work?

@mqus
Copy link

mqus commented Apr 30, 2022

Just to be clear: tests "failed" because I don't pay for CI. I have no reason to assume that anything didn't work. I am currently also not interested in rebasing when there is no chance of it getting merged.

The changes itself are very simple (the steps are reflected in the commits):

  1. move the migrations into a mysql subdirectory (so we can have a similar one for sqlite)
  2. copy the mysql stuff (I mainly copy+pasted the mysql code and fixed any errors that popped up, do a diff between the two directories if you're interested in the changes)
  3. copy the CI stuff for testing the mysql stuff into a sqlite equivalent.

If you have some minor experience with sqlite, rust and diesel (the ORM library) this is doable from scratch on a weekend.
When rebasing, you either do the same from scratch or try to see what changed since the last rebase and apply that. From a quick glance, nothing changed at the schema level, so there should only be steps 2/3 left and even there the changes are probably not much.

@mqus
Copy link

mqus commented Apr 30, 2022

Others are more than welcome to continue work and resolve the outstanding issues. [...]

Which outstanding issues do you refer to? If it's the comments on the code, I would welcome an answer from anyone familiar with the existing service or code. (A "review"). If its in regard to "cleaning up" or removing the duplicated code between the mysql and sqlite backends: I am still eager to do that, once the questions are resolved and I have a "go" from your side. I just figured it was easier to receive feedback now where the code is completely separate and "merge" it once it is functionally complete (maybe even in a separate PR for easier review).

@jrconlin
Copy link
Member

jrconlin commented May 2, 2022

Ah, sorry, I missed that.

When I get a chance, I will try to merge this PR with latest main and do a proper review.

@itsmejoeeey
Copy link

itsmejoeeey commented Sep 28, 2022

Also interested in this feature for the same reasons as Glandos.

Is there any update on the status of this issue?

@crozone
Copy link

crozone commented Oct 21, 2022

+1 for SQLite support, would be very nice to have for more minimal self-hosted installations.

I might try running #935 as a fork since it seems to be pretty much good to go.

@mqus
Copy link

mqus commented Oct 30, 2022

I might try running #935 as a fork since it seems to be pretty much good to go.

You can try, but by now, this branch is a bit outdated, esp. since syncstorage_rs seems to have a bit more parts which would need changes to support sqlite/pgsql (=the tokenserver afaict). I'm also not sure I understand the new layout, since there are now two migration folders(migrations and syncserver/src/tokenserver/migrations), which means two databases, but referenced from the same crate?

You will need to still use the old python server as well, if you're working with my branch and I'm not sure if that works on current firefox versions.

@mqus
Copy link

mqus commented Oct 30, 2022

I see that #1407 will probably improve things a lot, just going from the title. Maybe I'll try "from scratch" when this has landed :)

@Eragonfr
Copy link

Eragonfr commented Feb 5, 2023

#1407 has been merged, do you have an idea on how to start from that new architecture ?
I can try to help but I can't imagine how to add sqlite and postgres support.

@Fijxu
Copy link

Fijxu commented Apr 4, 2023

+1 For PSQL support and SQLite.

First, using 2 SQL servers just because some programs doesn't support X SQL server is not practical. And SQLite for little self-hosted instances for personal use, since they are not gonna use a lot of data and operations will not be like a big instance

Thanks!

@bcspragu
Copy link

In the same boat, looking to self-host with SQLite for simplicity. My Rust is decidedly mediocre, but happy to pick up the torch and give this a shot. Is there anything that needs to be done with #935, aside from a potentially nightmare-ish rebasing onto main again and actually running the CI?

@spacekitteh
Copy link

+1 for PSQL!

@DarkCat09
Copy link

Support for PostgreSQL would be great

@andrewille
Copy link

Any news on this? I really wish PostgreSQL was supported thus I can reuse my existing database. Setting up another database for this would probably not work for me as I have very few resources left. :/

@Eragonfr Eragonfr linked a pull request Feb 11, 2024 that will close this issue
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
13 Estimate - xxl - Complex, large effort, well defined (these should be broken down further) enhancement New feature or request good first bug Low priority, but valuable contributions
Projects
None yet
Development

Successfully merging a pull request may close this issue.