Upgrade to Ecto 3 #1984
Upgrade to Ecto 3 #1984
Conversation
All clickhouse tests are done with mocks. The adapter is needed so a connection can be established and we do not get Repo not started error. The postgres db won't ever be queried from ClickhouseRepo
lib/sanbase/insight/post.ex
Outdated
| @@ -61,7 +61,7 @@ defmodule Sanbase.Insight.Post do | |||
| on_delete: :delete_all | |||
| ) | |||
|
|
|||
| field(:published_at, :naive_datetime) | |||
| field(:published_at, :naive_datetime_usec) | |||
There was a problem hiding this comment.
is that required to fix datetime comparisons in tests ?
There was a problem hiding this comment.
I think i'll revert back to naive_datetime and change the tests so different insights have a full second difference in creation time
lib/sanbase/timeline/cursor.ex
Outdated
| @@ -4,14 +4,14 @@ defmodule Sanbase.Timeline.Cursor do | |||
| def filter_by_cursor(query, :before, datetime) do | |||
| from( | |||
| event in query, | |||
| where: event.inserted_at < ^datetime | |||
| where: event.inserted_at < ^DateTime.to_naive(datetime) | |||
There was a problem hiding this comment.
We have to convert to naive in order to user comparison with datetime field in query ?
There was a problem hiding this comment.
I am not sure and have to double-check
| end) | ||
| end | ||
| column("San balance", fn eth_account -> | ||
| case EthAccount.san_balance(eth_account) do |
There was a problem hiding this comment.
I think it returns: san_balance | :error.
Also it will make a call to parity service on opening every user in admin in dev env
There was a problem hiding this comment.
It's very very rare to open user ex_admin from dev. We already need a connection to some services using VPN, so parity is easy to enable as well
There was a problem hiding this comment.
the san_balance function returns :error instead of nil
- Add comments to functions - Add --skip-if-loaded flag to test alias - Make insights datetimes all :naive_datetime - Fix wrong @impl in MetricAdapter
Summary
Changed dependencies:
Clickhousex: https://github.com/IvanIvanoff/clickhousex
ClickhouseEcto: http://github.com/santiment/clickhouse_ecto/pull/6
ExAdmin: santiment/ex_admin#1
These dependencies are not in santiment master branches because this would break current builds (before this is merged). After merging this, testing on stage and deploying to prod we can safely move all dependencies to sentiment master branches.
Some useful links connected to ecto 2 -> 3 migrations when work on adapters is required
https://github.com/elixir-ecto/ecto/blob/master/CHANGELOG.md#adapter-changes
elixir-ecto/db_connection#167
https://elixirforum.com/t/how-much-work-to-update-an-ecto2-adapter-to-ecto3/18689/12