Skip to content

Conversation

@odarriba
Copy link
Contributor

@odarriba odarriba commented Aug 5, 2024

This change adds support for SQLite3 storage of errors and includes a new error_tracker_meta table to store metadata needed for ErrorTracker to function properly - like managing migration versions.

Aside from adding support to SQLite3, which was as easy as expected, we have also added a way to track migration versions that should work in any SQL database (although only PostgreSQL and SQLite3 are currently supported). That KV store table will also work in the future to store runtime configuration, for example.

In this change we are also migrating PostgreSQL systems to use this new table, which means migrating from using table comments to store migration version to use this new table. In order to do so, a new version of the schema (2) have been added and current systems need to add a new migration:

defmodule MyApp.Repo.Migrations.UpdateErrorTrackerToV2 do
  use Ecto.Migration

  def up, do: ErrorTracker.Migration.up(version: 2)
  def down, do: ErrorTracker.Migration.down(version: 2)
end

As for SQLite3 migrations, migrations start at V02 to maintain the same version IDs on all platforms.

--

Development script has also been updated and this feature can be tested using the new config flags that can be seen in dev.local.example.exs.

@odarriba odarriba added the enhancement New feature or request label Aug 5, 2024
@odarriba odarriba self-assigned this Aug 5, 2024
@odarriba odarriba changed the title WIP: Add SQLite3 support WIP: Add SQLite3 support ans meta table Aug 6, 2024
@odarriba odarriba changed the title WIP: Add SQLite3 support ans meta table Add SQLite3 support ans meta table Aug 6, 2024
@odarriba odarriba requested a review from crbelaus August 6, 2024 09:58
@crbelaus
Copy link
Contributor

crbelaus commented Aug 6, 2024

I've tried to update an existing test project to the v2 schema and it worked just fine. There seems to be an issue when rolling back though:

defmodule PhxNewPostgres.Repo.Migrations.UpdateErrorTrackerToV2 do
  use Ecto.Migration

  def up, do: ErrorTracker.Migration.up(version: 2)
  def down, do: ErrorTracker.Migration.down(version: 2)
end
➜ mix ecto.rollback -n 1

16:04:17.277 [info] == Running 20240806140311 PhxNewPostgres.Repo.Migrations.UpdateErrorTrackerToV2.down/0 forward

16:04:17.285 [info] drop table public.error_tracker_meta

16:04:17.286 [info] execute "COMMENT ON TABLE \"public\".error_tracker_errors IS '1'"

16:04:17.287 [info] execute "INSERT INTO public.error_tracker_meta (key, value) VALUES ('migration_version', '1')\nON CONFLICT (key) DO UPDATE SET value = '1'\n"
** (Postgrex.Error) ERROR 42P01 (undefined_table) relation "public.error_tracker_meta" does not exist

    query: INSERT INTO public.error_tracker_meta (key, value) VALUES ('migration_version', '1')
ON CONFLICT (key) DO UPDATE SET value = '1'

    (ecto_sql 3.11.3) lib/ecto/adapters/sql.ex:1054: Ecto.Adapters.SQL.raise_sql_call_error/1
    (elixir 1.16.3) lib/enum.ex:1700: Enum."-map/2-lists^map/1-1-"/2
    (ecto_sql 3.11.3) lib/ecto/adapters/sql.ex:1161: Ecto.Adapters.SQL.execute_ddl/4
    (ecto_sql 3.11.3) lib/ecto/migration/runner.ex:348: Ecto.Migration.Runner.log_and_execute_ddl/3
    (elixir 1.16.3) lib/enum.ex:1700: Enum."-map/2-lists^map/1-1-"/2
    (elixir 1.16.3) lib/enum.ex:1700: Enum."-map/2-lists^map/1-1-"/2
    (ecto_sql 3.11.3) lib/ecto/migration/runner.ex:311: Ecto.Migration.Runner.perform_operation/3
    (stdlib 5.0.2) timer.erl:270: :timer.tc/2

crbelaus

This comment was marked as duplicate.

Copy link
Contributor

@crbelaus crbelaus left a comment

Choose a reason for hiding this comment

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

I've just tested this on both PostgreSQL and Sqlite3 and works great. I very much prefer using the meta table instead of relying on table comments.

There is a problem when rolling back the new Postgres migration though.

@odarriba
Copy link
Contributor Author

odarriba commented Aug 6, 2024

@crbelaus I have updated the change and also unified the migration code for SQL databases, so most of the logic is now common on both migrators.

I have also fixed the issue with the rollback, which was because of the migration of the table in PostgreSQL and not supporting the comment version anymore.

Please, let me know how you see it now 🤗

Copy link
Contributor

@crbelaus crbelaus left a comment

Choose a reason for hiding this comment

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

Tested again and it is working great!

@odarriba odarriba merged commit 88f27a8 into main Aug 6, 2024
@odarriba odarriba deleted the add-sqlite-support branch August 6, 2024 17:52
@crbelaus crbelaus changed the title Add SQLite3 support ans meta table Add SQLite3 support and meta table Aug 7, 2024
@crbelaus crbelaus added this to the v0.2.0 milestone Aug 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants