Refactor: Split dashboard-api queries & migrations#2168
Refactor: Split dashboard-api queries & migrations#2168ben-fornefeld wants to merge 5 commits intomainfrom
Conversation
…//github.com/e2b-dev/infra into refactor/split-dashboard-db-responsibilities
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
.../dashboard/migrations/20260318140000_dashboard_add_env_defaults_and_team_profile_picture.sql
Show resolved
Hide resolved
| ClusterID pgtype.UUID | ||
| SandboxSchedulingLabels []string | ||
| Slug string | ||
| } |
There was a problem hiding this comment.
The testutils Team struct is missing the ProfilePictureUrl field added to public.teams by the dashboard migration. The testutils sqlc config in sqlc.yaml includes migrations and schema but not pkg/dashboard/migrations, so the new column is not picked up. If any test scans all columns from teams into this struct it will fail at runtime due to column count mismatch.
| telemetry.SetAttributes(ctx, telemetry.WithTeamID(teamID.String()), telemetry.WithBuildID(buildId.String())) | ||
|
|
||
| row, err := s.db.GetBuildInfoByTeamAndBuildID(ctx, queries.GetBuildInfoByTeamAndBuildIDParams{ | ||
| row, err := s.db.GetBuildInfoByTeamAndBuildID(ctx, dashboardqueries.GetBuildInfoByTeamAndBuildIDParams{ |
There was a problem hiding this comment.
I know these queries are called only in the dashboard-api, but lets keep them colocated to the database (source), rather than usage. -> they should stay in core
| expectedDashboardMigration = 0 | ||
| } | ||
|
|
||
| err = sqlcdb.CheckMigrationVersionWithTable(ctx, config.PostgresConnectionString, "_migrations", expectedCoreMigration) |
There was a problem hiding this comment.
the migration table should not be necessary to provide here, correct? Or can we at least reuse the const for the infra?
There was a problem hiding this comment.
why not necessary?
There was a problem hiding this comment.
because its the default one
| conn *pgxpool.Pool | ||
| } | ||
|
|
||
| func NewClient(ctx context.Context, databaseURL string, options ...pool.Option) (*Client, error) { |
There was a problem hiding this comment.
why do we need this copied here?
| l.Fatal(ctx, "failed to check core migration version", zap.Error(err)) | ||
| } | ||
|
|
||
| err = sqlcdb.CheckMigrationVersionWithTable(ctx, config.PostgresConnectionString, "_migrations_dashboard", expectedDashboardMigration) |
There was a problem hiding this comment.
lets move _migrations_dashboard to const
3 participants
No description provided.