-
Notifications
You must be signed in to change notification settings - Fork 157
Feat; Create new table for collector configuration #2157
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
base: master
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -285,6 +285,17 @@ static MIGRATIONS: &[&str] = &[ | |
alter table pstat_series drop constraint test_case; | ||
alter table pstat_series add constraint test_case UNIQUE(crate, profile, scenario, backend, target, metric); | ||
"#, | ||
r#"CREATE EXTENSION IF NOT EXISTS "uuid-ossp";"#, | ||
r#" | ||
CREATE TABLE IF NOT EXISTS collector_config ( | ||
id UUID PRIMARY KEY, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We will have probably max. 4? collectors in the near future 😅 So I think that UUIDs are overkill. My experience with messing with Postgres extensions is that it can go wrong, I would prefer to avoid this and just use good ol' autoincremented IDs. On the other hand, I'd like to assign human-readable names to the collectors, for easier debugging. So please add a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
target TEXT NOT NULL, | ||
date_added TIMESTAMPTZ DEFAULT NOW() NOT NULL, | ||
last_heartbeat_at TIMESTAMPTZ, | ||
benchmark_set UUID NOT NULL, | ||
Jamesbarford marked this conversation as resolved.
Show resolved
Hide resolved
|
||
is_active BOOLEAN DEFAULT FALSE NOT NULL | ||
); | ||
"#, | ||
]; | ||
|
||
#[async_trait::async_trait] | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -404,6 +404,18 @@ static MIGRATIONS: &[Migration] = &[ | |
alter table pstat_series_with_target rename to pstat_series; | ||
"#, | ||
), | ||
Migration::without_foreign_key_constraints( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if we should "delay" the tables on the SQLite side until we finish the Postgres implementation? Just to potentially avoid needless migrations (i.e. Postgres could serve as a "testbed" for the DB). I'm not even sure if we'll ever need this table on the SQLite side, tbh. |
||
r#" | ||
CREATE TABLE IF NOT EXISTS collector_config ( | ||
id TEXT PRIMARY KEY, | ||
target TEXT NOT NULL, | ||
date_added TIMESTAMP DEFAULT CURRENT_TIMESTAMP NOT NULL, | ||
last_heartbeat_at TIMESTAMP, | ||
benchmark_set TEXT NOT NULL, | ||
is_active BOOLEAN DEFAULT FALSE NOT NULL | ||
); | ||
"#, | ||
), | ||
]; | ||
|
||
#[async_trait::async_trait] | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please update this with
name
? Thanks!There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
34b927d