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

Modify todo example to use postgres and diesel_async #2648

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

Conversation

ThouCheese
Copy link
Contributor

@ThouCheese ThouCheese commented Nov 21, 2023

As requested. Notable: diesel_migrations does not work with diesel_async, so I establish a separate connection in the migration function.

I don't know if there is a postgres database present during CI, so I called it "epic_todo_database" to emphasise that that section is free form

@ThouCheese ThouCheese force-pushed the fix/diesel_async_examples branch 3 times, most recently from ae03d1f to 6a6b53c Compare November 21, 2023 15:20
@SergioBenitez
Copy link
Member

What's the status of this? We were looking into having a postgres DB available when the CI runs to merge this. @ELD Were you working on that?

It would be nice if this example could run without a postgres DB as well, however. Perhaps we can have a dynamic switch for the database based on a profile and implement it for both backends in the same example?

@ELD
Copy link
Member

ELD commented Dec 24, 2023

@SergioBenitez I was working on getting the CI piece done. I got it running on macOS CI, but not Windows or Linux (these are being particularly tricky). It's also difficult to make this resilient to GHA rolling out a new machine images as the current ones have Postgres pre-installed, but future ones may not (and that complicates the configuration).

I haven't had time to revisit it since this time of year tends to be pretty busy. I'm hoping to get back to it after the Christmas holiday and finish this off.

I like the idea of a profile flag for whether we use something like SQLite in sync mode or Postgres in diesel-async. Not sure what that implementation would look like, though.

Definitely think we can push this over the line by the early part of the new year.

@ELD
Copy link
Member

ELD commented Dec 29, 2023

Alright, I have Postgres working on Linux and macOS now. Just down to Windows: https://github.com/ELD/Rocket/actions/runs/7353097692

- Start the preinstalled postgres service on the all executors
  - Note: This will work until macos-13 becomes the default since it's
    seemingly not installed on those runner images
- Create `rocket_runner` user
- Create the `epic_todo_database` DB
- Update SQL migrations to match Postgres dialect (different from SQLite
  or MySQL)
- Update user/pass configuration for Postgres user/db
@ELD
Copy link
Member

ELD commented Dec 29, 2023

Finished off the CI work for Windows and Linux along with some vcpkg changes to pull in the latest versions of libpq for tests (and caching via actions/cache). We can vendor the vcpkg packages, but the actions/cache solution seemingly works well. It'll take longer, though, when certain build deps increase their version.

We just need @ThouCheese (no rush on this, just tagging for the sake of visibility) to merge the upstream PR to then get a final review in this PR :)

Fix CI for Rocket diesel async example
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.

3 participants