-
Notifications
You must be signed in to change notification settings - Fork 31
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
allow the configuration of executors memory during deploy #646
Conversation
@@ -145,21 +147,37 @@ export async function deployAppCode( | |||
} else { | |||
logger.info("Time travel is disabled for this application"); | |||
} | |||
const ret = await registerApp(userDBName, host, enableTimeTravel, appName); | |||
const ret = await registerApp(userDBName, host, enableTimeTravel, appName, executorsMemoryMib); |
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.
Set executors RAM when registering
logger.warn( | ||
`Application ${chalk.bold(appName)} is deployed with database instance ${chalk.bold(appRegistered.PostgresInstanceName)}. Ignoring the provided database instance name ${chalk.bold( | ||
userDBName | ||
)}.` | ||
); | ||
} | ||
|
||
// Make sure the app database is the same. | ||
if (appRegistered.ApplicationDatabaseName && (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.`); | ||
if (appRegistered.ApplicationDatabaseName && 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.` | ||
); |
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.
lint
// Check if the user wants to update the executors memory | ||
if (executorsMemoryMib && appRegistered.ExecutorsMemoryMib !== executorsMemoryMib) { | ||
const ret = await updateApp(host, appName, executorsMemoryMib, userCredentials); | ||
if (ret !== 0) { | ||
return 1; | ||
} | ||
} |
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.
If the users provides a desired RAM amount, and this amount is different from what the application record has, then update the application record.
0666106
to
ceccebe
Compare
import axios, { AxiosError } from "axios"; | ||
import { handleAPIErrors, getCloudCredentials, getLogger, isCloudAPIErrorResponse, retrieveApplicationName, DBOSCloudCredentials } from "../cloudutils.js"; | ||
|
||
export async function updateApp(host: string, appName?: string, executorsMemoryMib?: number, userCredentials?: DBOSCloudCredentials): Promise<number> { |
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.
Let's make this a separate command, not part of deploy
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.
done
…ements -- not as part of deploy
TODO: