-
Notifications
You must be signed in to change notification settings - Fork 23
add database retry #1447
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: next-4
Are you sure you want to change the base?
add database retry #1447
Changes from all commits
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 |
|---|---|---|
|
|
@@ -5,9 +5,15 @@ import { | |
| OverrideEnvConfig, | ||
| TEST_ENV_CONFIG_PATH, | ||
| buildEnvOverrideConfig, | ||
| setupEnvironment | ||
| setupEnvironment, | ||
| tearDownEnvironment | ||
| } from '../utils/utils.js' | ||
| import { ENVIRONMENT_VARIABLES } from '../../utils/constants.js' | ||
| import { | ||
| DEFAULT_DB_INIT_MAX_ATTEMPTS, | ||
| DEFAULT_DB_INIT_MAX_RETRY_DELAY, | ||
| DEFAULT_DB_INIT_RETRY_DELAY | ||
| } from '../../utils/config/constants.js' | ||
|
|
||
| let config: OceanNodeConfig | ||
| describe('Should validate configuration from JSON', () => { | ||
|
|
@@ -61,6 +67,94 @@ describe('Should validate configuration from JSON', () => { | |
| }) | ||
| }) | ||
|
|
||
| describe('Should validate database init retry configuration', () => { | ||
| const DB_ENV_VARS = [ENVIRONMENT_VARIABLES.DB_TYPE, ENVIRONMENT_VARIABLES.DB_URL] | ||
| const DB_ENV_VALUES = ['typesense', 'http://localhost:8108/?apiKey=xyz'] | ||
|
|
||
| // returns the config built with the given DB_INIT_* values, or the thrown error | ||
| async function configWith(values: { | ||
| attempts?: string | ||
| delay?: string | ||
| maxDelay?: string | ||
| }): Promise<{ config?: OceanNodeConfig; error?: Error }> { | ||
| const envVars = [...DB_ENV_VARS] | ||
| const envValues = [...DB_ENV_VALUES] | ||
| if (values.attempts !== undefined) { | ||
| envVars.push(ENVIRONMENT_VARIABLES.DB_INIT_MAX_ATTEMPTS) | ||
| envValues.push(values.attempts) | ||
| } | ||
| if (values.delay !== undefined) { | ||
| envVars.push(ENVIRONMENT_VARIABLES.DB_INIT_RETRY_DELAY) | ||
| envValues.push(values.delay) | ||
| } | ||
| if (values.maxDelay !== undefined) { | ||
| envVars.push(ENVIRONMENT_VARIABLES.DB_INIT_MAX_RETRY_DELAY) | ||
| envValues.push(values.maxDelay) | ||
| } | ||
| // setupEnvironment() reloads the configuration itself, so an invalid value already throws | ||
| // there — it must be inside the try. The override array records the original values as it | ||
| // goes, so it is still usable for the teardown after a throw. | ||
| const overrides = buildEnvOverrideConfig(envVars, envValues) | ||
| try { | ||
| await setupEnvironment(TEST_ENV_CONFIG_PATH, overrides) | ||
| return { config: await getConfiguration(true) } | ||
| } catch (error) { | ||
| return { error } | ||
| } finally { | ||
| await tearDownEnvironment(overrides) | ||
| } | ||
|
Comment on lines
+94
to
+105
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. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Restore cached environment values during test cleanup.
Update the environment helper to restore As per coding guidelines, “Do not mutate Also applies to: 152-155 🤖 Prompt for AI AgentsSource: Coding guidelines |
||
| } | ||
|
|
||
| it('should apply the documented defaults when the variables are not set', async () => { | ||
| const { config: conf, error } = await configWith({}) | ||
| expect(error).to.be.equal(undefined) | ||
| expect(conf.dbInitMaxAttempts).to.be.equal(DEFAULT_DB_INIT_MAX_ATTEMPTS) | ||
| expect(conf.dbInitRetryDelay).to.be.equal(DEFAULT_DB_INIT_RETRY_DELAY) | ||
| expect(conf.dbInitMaxRetryDelay).to.be.equal(DEFAULT_DB_INIT_MAX_RETRY_DELAY) | ||
| }) | ||
|
|
||
| it('should coerce the environment variables to numbers', async () => { | ||
| const { config: conf, error } = await configWith({ | ||
| attempts: '3', | ||
| delay: '500', | ||
| maxDelay: '5000' | ||
| }) | ||
| expect(error).to.be.equal(undefined) | ||
| expect(conf.dbInitMaxAttempts).to.be.equal(3) | ||
| expect(conf.dbInitRetryDelay).to.be.equal(500) | ||
| expect(conf.dbInitMaxRetryDelay).to.be.equal(5000) | ||
| }) | ||
|
|
||
| // 0 attempts would never enter the retry loop, so Database.init() would not be called at | ||
| // all and the node would silently boot without any database | ||
| it('should refuse to start when the number of attempts is zero or negative', async () => { | ||
| for (const attempts of ['0', '-1']) { | ||
| const { config: conf, error } = await configWith({ attempts }) | ||
| expect(conf, `attempts=${attempts} should not produce a config`).to.be.equal( | ||
| undefined | ||
| ) | ||
| expect(error?.message).to.be.equal('Configuration validation failed') | ||
| } | ||
| }) | ||
|
|
||
| it('should refuse to start on a non-numeric value', async () => { | ||
| const { config: conf, error } = await configWith({ attempts: 'abc' }) | ||
| expect(conf).to.be.equal(undefined) | ||
| expect(error?.message).to.be.equal('Configuration validation failed') | ||
| }) | ||
|
|
||
| it('should refuse to start when a delay is zero', async () => { | ||
| const { config: conf, error } = await configWith({ delay: '0' }) | ||
| expect(conf).to.be.equal(undefined) | ||
| expect(error?.message).to.be.equal('Configuration validation failed') | ||
| }) | ||
|
|
||
| after(() => { | ||
| delete process.env.CONFIG_PATH | ||
| delete process.env.PRIVATE_KEY | ||
| }) | ||
| }) | ||
|
|
||
| describe('Should validate P2P config from environment variables', () => { | ||
| let config: OceanNodeConfig | ||
| let envOverrides: OverrideEnvConfig[] | ||
|
|
||
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.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
Repository: oceanprotocol/ocean-node
Length of output: 6573
🏁 Script executed:
Repository: oceanprotocol/ocean-node
Length of output: 50381
🏁 Script executed:
Repository: oceanprotocol/ocean-node
Length of output: 50381
🏁 Script executed:
Repository: oceanprotocol/ocean-node
Length of output: 14302
Preserve the optional database path and expose the nullable result.
Database.initreturnsPromise<Database | null>, andOceanNode.getInstanceaccepts an optional database. ChangeinitDatabaseWithRetryanddbconnto useDatabase | nullinstead of hiding the nullable result.🤖 Prompt for AI Agents