Conversation
Pulling changes from Burokratt WIP to rootcodelabs/RAG-Module wip
Get update from RAG-201-Fix into encrypt-llm-keys
update cron manager vault script
Streaming response formatting
Encrypt llm keys
…G-Module into encrypt-llm-keys
Sync rootcodelabs/RAG-Module wip with buerokratt/RAG-Module wip
… encrypt-llm-keys
…G-Module into encrypt-llm-keys
Refactor docker-compose-ec2.ym l file with new vault agent containers
… intents-db-schema
There was a problem hiding this comment.
Pull request overview
This PR adds a new database schema for a services table to support service management functionality in the RAG search system. The services table is designed to store service definitions with intent classification data for LLM-based routing.
Changes:
- Added new Liquibase migration file
rag-search-script-v5-services.sqlthat defines theservicestable and required ENUM types - Updated
master.ymlto include the new services migration before the prompt configuration migration
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 10 comments.
| File | Description |
|---|---|
| DSL/Liquibase/master.yml | Added include directive for the new services migration file |
| DSL/Liquibase/changelog/rag-search-script-v5-services.sql | New migration defining the services table with fields for service metadata, intent classification, configuration, and timestamps |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ruuter_type ruuter_request_type DEFAULT 'GET', -- ENUM: 'GET' or 'POST' | ||
| current_state service_state DEFAULT 'draft', -- ENUM: 'active', 'inactive', 'draft' | ||
| is_common BOOLEAN NOT NULL DEFAULT false, | ||
| deleted BOOLEAN NOT NULL DEFAULT false, |
There was a problem hiding this comment.
The deleted field uses uppercase BOOLEAN (line 14) while other occurrences in the same file use lowercase (line 13). For consistency within this file, the casing should be uniform. The existing codebase also uses uppercase TRUE/FALSE (line 14) which is consistent.
| deleted BOOLEAN NOT NULL DEFAULT false, | |
| deleted boolean NOT NULL DEFAULT false, |
| @@ -0,0 +1,32 @@ | |||
| CREATE TABLE services ( | |||
| -- Primary key | |||
| id BIGSERIAL PRIMARY KEY, | |||
There was a problem hiding this comment.
The table is missing a primary key constraint name. Looking at other tables in the codebase (e.g., rag-search-script-v2-user-management.sql:19,31, rag-search-script-v3-configuration.sql:10), primary keys are explicitly named using "CONSTRAINT [tablename]_pkey PRIMARY KEY (id)". For consistency, this should be "CONSTRAINT services_pkey PRIMARY KEY" instead of just "PRIMARY KEY".
| current_state service_state DEFAULT 'draft', -- ENUM: 'active', 'inactive', 'draft' | ||
| is_common BOOLEAN NOT NULL DEFAULT false, | ||
| deleted BOOLEAN NOT NULL DEFAULT false, |
There was a problem hiding this comment.
Based on the schema design, queries are likely to filter by current_state (active/inactive/draft) and deleted (soft delete pattern). However, there are no indexes defined for these columns. If the table grows large, queries filtering by these fields (e.g., SELECT * FROM services WHERE deleted = false AND current_state = 'active') may have poor performance. Consider adding indexes on these columns, either individually or as a composite index.
| CREATE TABLE services ( | ||
| -- Primary key | ||
| id BIGSERIAL PRIMARY KEY, | ||
|
|
||
| -- Basic service information | ||
| name TEXT NOT NULL, | ||
| description TEXT NOT NULL, | ||
| service_id TEXT NOT NULL UNIQUE, | ||
|
|
||
| -- Service classification | ||
| ruuter_type ruuter_request_type DEFAULT 'GET', -- ENUM: 'GET' or 'POST' | ||
| current_state service_state DEFAULT 'draft', -- ENUM: 'active', 'inactive', 'draft' | ||
| is_common BOOLEAN NOT NULL DEFAULT false, | ||
| deleted BOOLEAN NOT NULL DEFAULT false, | ||
|
|
||
| -- Intent classification data (for LLM) | ||
| slot TEXT NOT NULL DEFAULT '', | ||
| entities text[] NOT NULL DEFAULT '{}', | ||
| examples text[] NOT NULL DEFAULT '{}', | ||
|
|
||
| -- Service configuration | ||
| structure JSON NOT NULL DEFAULT '{}', | ||
| endpoints JSON NOT NULL DEFAULT '[]', | ||
|
|
||
| -- Timestamps | ||
| created_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), | ||
| updated_at TIMESTAMPTZ NOT NULL DEFAULT NOW() | ||
| ); | ||
|
|
||
| -- Custom types used: | ||
| CREATE TYPE ruuter_request_type AS ENUM ('GET', 'POST'); | ||
| CREATE TYPE service_state AS ENUM ('active', 'inactive', 'draft'); No newline at end of file |
There was a problem hiding this comment.
The custom ENUM types (ruuter_request_type and service_state) are being created at lines 31-32, but they are referenced in the CREATE TABLE statement at lines 11-12. In SQL, types must be defined before they are used. This will cause the table creation to fail with an error about undefined types. The CREATE TYPE statements should be moved to appear before the CREATE TABLE statement.
| @@ -0,0 +1,32 @@ | |||
| CREATE TABLE services ( | |||
There was a problem hiding this comment.
The new migration file is missing the Liquibase formatted SQL header and changeset declarations. Looking at other migration files in this codebase (e.g., rag-search-script-v2-user-management.sql, rag-search-script-v3-configuration.sql, rag-search-script-v5-prompt-config.sql), they all start with "-- liquibase formatted sql" followed by "-- changeset [author]:[changeset-id]" comments. This file should follow the same pattern for consistency and proper Liquibase tracking.
| @@ -0,0 +1,32 @@ | |||
| CREATE TABLE services ( | |||
| -- Primary key | |||
| id BIGSERIAL PRIMARY KEY, | |||
There was a problem hiding this comment.
The id field uses BIGSERIAL, but other tables in this codebase use "BIGINT NOT NULL GENERATED BY DEFAULT AS IDENTITY" (see rag-search-script-v3-configuration.sql:5 and rag-search-script-v5-prompt-config.sql:5). While BIGSERIAL is functionally similar, using "BIGINT NOT NULL GENERATED BY DEFAULT AS IDENTITY" would be more consistent with the established pattern in this codebase.
| -- Basic service information | ||
| name TEXT NOT NULL, | ||
| description TEXT NOT NULL, | ||
| service_id TEXT NOT NULL UNIQUE, |
There was a problem hiding this comment.
The service_id field is defined as UNIQUE but there's no explicit index name provided. While PostgreSQL will create an implicit index for UNIQUE constraints, for consistency with the naming conventions seen in this codebase and for explicit control, consider adding an explicitly named unique constraint or index. However, this is a minor point as the UNIQUE constraint will function correctly.
| created_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), | ||
| updated_at TIMESTAMPTZ NOT NULL DEFAULT NOW() |
There was a problem hiding this comment.
The timestamp fields use TIMESTAMPTZ (line 26-27) while other tables in this codebase consistently use "TIMESTAMP WITH TIME ZONE" (see rag-search-script-v2-user-management.sql:18,30 and rag-search-script-v3-configuration.sql:8). While TIMESTAMPTZ is an alias for TIMESTAMP WITH TIME ZONE in PostgreSQL, using the full form would be more consistent with the established convention in this codebase.
| created_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), | ||
| updated_at TIMESTAMPTZ NOT NULL DEFAULT NOW() |
There was a problem hiding this comment.
The timestamp defaults use NOW() (lines 26-27) while other tables in this codebase use CURRENT_TIMESTAMP (see rag-search-script-v2-user-management.sql:18,30 and rag-search-script-v3-configuration.sql:8). While both are functionally equivalent in PostgreSQL, using CURRENT_TIMESTAMP would be more consistent with the established pattern in this codebase.
| @@ -0,0 +1,32 @@ | |||
| CREATE TABLE services ( | |||
There was a problem hiding this comment.
The table name "services" is not schema-qualified, unlike other tables in this codebase which use "public." prefix (see rag-search-script-v2-user-management.sql:7,22,26, rag-search-script-v3-configuration.sql:4, and rag-search-script-v5-prompt-config.sql:4). For consistency with the established convention, this should be "CREATE TABLE public.services".
3 participants
No description provided.