Skip to content
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

Simplify Database Configuration #653

Merged
merged 3 commits into from
Oct 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions dbos-config.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,7 @@
"hostname",
"port",
"username",
"password",
"app_db_name"
"password"
]
},
"telemetry": {
Expand Down
26 changes: 1 addition & 25 deletions packages/create/init.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,12 @@ import fs from 'fs'
import { execSync } from 'child_process'
import validator from 'validator';
import { fileURLToPath } from 'url';
import YAML from "yaml";

interface CopyOption {
rename?: (basename: string) => string;
}

const identity = (x: string) => x;
const dbosConfigFilePath = "dbos-config.yaml";

export const copy = async (
src: string,
Expand Down Expand Up @@ -55,7 +53,7 @@ function isValidApplicationName(appName: string): boolean {
}

const filesAllowedInEmpty = ['.gitignore', 'readme.md'];
const pathPrefixesAllowedInEmpty = ['.']; // Directories that start with
const pathPrefixesAllowedInEmpty = ['.']; // Directories that start with

function dirHasStuffInIt(pathName: string): boolean {
if (!fs.existsSync(pathName)) {
Expand Down Expand Up @@ -100,27 +98,6 @@ function mergeGitIgnore(existingGISet: Set<string>, templateGISet: Set<string>):
return joined.replaceAll('\n#','\n\n#');
}

function updateAppConfig(configFilePath: string, appName: string): void {
try {
const content = fs.readFileSync(configFilePath, 'utf-8');
const configFile = YAML.parseDocument(content);

// Change the app_db_name field to the sanitized appName, replacing dashes with underscores
let appDBName = appName.toLowerCase().replaceAll('-', '_');
if (appDBName.match(/^\d/)) {
appDBName = "_" + appDBName; // Append an underscore if the name starts with a digit
}
configFile.setIn(['database', 'app_db_name'], appDBName);
fs.writeFileSync(configFilePath, configFile.toString());
} catch (e) {
if (e instanceof Error) {
throw new Error(`Failed to load config from ${configFilePath}: ${e.message}`);
} else {
throw e;
}
}
}

function mergeGitignoreFiles(existingFilePath: string, templateFilePath: string, outputFilePath: string): void {
const existingSet = loadGitignoreFile(existingFilePath);
const templateSet = loadGitignoreFile(templateFilePath);
Expand Down Expand Up @@ -157,6 +134,5 @@ export async function init(appName: string, templateName: string) {
execSync("npm install --no-fund --save @dbos-inc/dbos-sdk@latest --loglevel=error", {cwd: appName, stdio: 'inherit'})
execSync("npm i --no-fund --loglevel=error", {cwd: appName, stdio: 'inherit'})
execSync("npm install --no-fund --save-dev @dbos-inc/dbos-cloud@latest", {cwd: appName, stdio: 'inherit'})
updateAppConfig(path.join(appName, dbosConfigFilePath), appName);
console.log("Application initialized successfully!")
}
1 change: 0 additions & 1 deletion packages/create/templates/hello-drizzle/dbos-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ database:
port: 5432
username: postgres
password: ${PGPASSWORD}
app_db_name: hello_drizzle
connectionTimeoutMillis: 3000
app_db_client: drizzle
migrate:
Expand Down
1 change: 0 additions & 1 deletion packages/create/templates/hello-prisma/dbos-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ database:
hostname: 'localhost'
port: 5432
username: 'postgres'
app_db_name: 'hello_prisma'
password: ${PGPASSWORD}
connectionTimeoutMillis: 3000
app_db_client: prisma
Expand Down
1 change: 0 additions & 1 deletion packages/create/templates/hello-typeorm/dbos-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ database:
hostname: 'localhost'
port: 5432
username: 'postgres'
app_db_name: 'hello_typeorm'
password: ${PGPASSWORD}
connectionTimeoutMillis: 3000
app_db_client: typeorm
Expand Down
1 change: 0 additions & 1 deletion packages/create/templates/hello/dbos-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ database:
port: 5432
username: postgres
password: ${PGPASSWORD}
app_db_name: hello
connectionTimeoutMillis: 3000
app_db_client: knex
migrate:
Expand Down
3 changes: 1 addition & 2 deletions packages/dbos-cloud/applications/deploy-app-code.ts
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,6 @@ export async function deployAppCode(
if (userDBName === "") {
return 1;
}
logger.info(`Loaded application database name from ${dbosConfigFilePath}: ${dbosConfig.database.app_db_name}`);

// Register the app
if (enableTimeTravel) {
Expand All @@ -158,7 +157,7 @@ export async function deployAppCode(
}

// Make sure the app database is the same.
if (appRegistered.ApplicationDatabaseName && (dbosConfig.database.app_db_name !== appRegistered.ApplicationDatabaseName)) {
if (appRegistered.ApplicationDatabaseName && dbosConfig.database.app_db_name && (dbosConfig.database.app_db_name !== appRegistered.ApplicationDatabaseName)) {
logger.error(`Application ${chalk.bold(appName)} is deployed with app_db_name ${chalk.bold(appRegistered.ApplicationDatabaseName)}, but ${dbosConfigFilePath} specifies ${chalk.bold(dbosConfig.database.app_db_name)}. Please update the app_db_name field in ${dbosConfigFilePath} to match the database name.`);
return 1;
}
Expand Down
36 changes: 30 additions & 6 deletions src/dbos-runtime/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ import { writeFileSync } from "fs";
import Ajv, { ValidateFunction } from 'ajv';
import path from "path";
import validator from "validator";
import fs from "fs";



export const dbosConfigFilePath = "dbos-config.yaml";
const dbosConfigSchemaPath = path.join(findPackageRoot(__dirname), 'dbos-config.schema.json');
Expand Down Expand Up @@ -95,9 +98,30 @@ export function writeConfigFile(configFile: YAML.Document, configFilePath: strin
}
}

export function retrieveApplicationName(configFile: ConfigFile): string {
let appName = configFile.name;
if (appName !== undefined) {
return appName
}
const packageJson = JSON.parse(fs.readFileSync(path.join(process.cwd(), "package.json")).toString()) as { name: string };
appName = packageJson.name;
if (appName === undefined) {
throw new DBOSInitializationError("Error: cannot find a valid package.json file. Please run this command in an application root directory.");
}
return appName;
}

export function constructPoolConfig(configFile: ConfigFile) {
const databaseName = configFile.database.local_suffix === true ? `${configFile.database.app_db_name}_local` : configFile.database.app_db_name
const poolConfig: PoolConfig = {
let databaseName: string | undefined = configFile.database.app_db_name;
if (databaseName === undefined) {
const appName = retrieveApplicationName(configFile)
databaseName = appName.toLowerCase().replaceAll('-', '_');
if (databaseName.match(/^\d/)) {
databaseName = "_" + databaseName; // Append an underscore if the name starts with a digit
}
}
databaseName = configFile.database.local_suffix === true ? `${databaseName}_local` : databaseName
const poolConfig: PoolConfig = {
host: configFile.database.hostname,
port: configFile.database.port,
user: configFile.database.username,
Expand Down Expand Up @@ -192,16 +216,16 @@ export function parseConfigFile(cliOptions?: ParseOptions, useProxy: boolean = f
throw new DBOSInitializationError(`${configFilePath} specifies invalid language ${configFile.language}`)
}

if (!isValidDBname(configFile.database.app_db_name)) {
throw new DBOSInitializationError(`${configFilePath} specifies invalid app_db_name ${configFile.database.app_db_name}. Must be between 3 and 31 characters long and contain only lowercase letters, underscores, and digits (cannot begin with a digit).`);
}

/*******************************/
/* Handle user database config */
/*******************************/

const poolConfig = constructPoolConfig(configFile);

if (!isValidDBname(poolConfig.database!)) {
throw new DBOSInitializationError(`${configFilePath} specifies invalid app_db_name ${configFile.database.app_db_name}. Must be between 3 and 31 characters long and contain only lowercase letters, underscores, and digits (cannot begin with a digit).`);
}

/***************************/
/* Handle telemetry config */
/***************************/
Expand Down
35 changes: 33 additions & 2 deletions tests/dbos-runtime/config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -129,10 +129,9 @@ describe("dbos-config", () => {
process.env.PGPASSWORD = dbPassword;
});

test("config file is missing app database name", () => {
test("config file is missing hostname", () => {
const localMockDBOSConfigYamlString = `
database:
hostname: 'some host'
port: 1234
username: 'some user'
password: \${PGPASSWORD}
Expand Down Expand Up @@ -248,6 +247,22 @@ describe("dbos-config", () => {
expect(dbosConfig.poolConfig.ssl).toEqual({ rejectUnauthorized: false });
});

test("config works without app_db_name", async () => {
const localMockDBOSConfigYamlString = `
name: some-app
database:
hostname: 'localhost'
port: 1234
username: 'some user'
password: \${PGPASSWORD}
`;
jest.restoreAllMocks();
jest.spyOn(utils, "readFileSync").mockReturnValue(localMockDBOSConfigYamlString);
const [dbosConfig, _dbosRuntimeConfig]: [DBOSConfig, DBOSRuntimeConfig] = parseConfigFile(mockCLIOptions);
const poolConfig = dbosConfig.poolConfig;
expect(poolConfig.database).toBe("some_app");
});


test("local_suffix works", async () => {
const localMockDBOSConfigYamlString = `
Expand All @@ -273,6 +288,22 @@ describe("dbos-config", () => {
expect(dbosConfig.poolConfig.ssl).toBe(false);
});

test("local_suffix works without app_db_name", async () => {
const localMockDBOSConfigYamlString = `
name: some-app
database:
hostname: 'localhost'
port: 1234
username: 'some user'
password: \${PGPASSWORD}
local_suffix: true
`;
jest.restoreAllMocks();
jest.spyOn(utils, "readFileSync").mockReturnValue(localMockDBOSConfigYamlString);
const [dbosConfig, _dbosRuntimeConfig]: [DBOSConfig, DBOSRuntimeConfig] = parseConfigFile(mockCLIOptions);
const poolConfig = dbosConfig.poolConfig;
expect(poolConfig.database).toBe("some_app_local");
});

test("ssl defaults off for localhost", async () => {
const localMockDBOSConfigYamlString = `
Expand Down
2 changes: 0 additions & 2 deletions tests/dbos-runtime/runtime.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,6 @@ database:
port: 5432
username: 'postgres'
password: \${PGPASSWORD}
app_db_name: 'hello'
connectionTimeoutMillis: 3000
app_db_client: 'knex'
runtimeConfig:
Expand Down Expand Up @@ -176,7 +175,6 @@ database:
port: 5432
username: 'postgres'
password: \${PGPASSWORD}
app_db_name: 'hello'
connectionTimeoutMillis: 3000
app_db_client: 'knex'
runtimeConfig:
Expand Down