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.

Sign up for GitHub

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

Jump to bottom

Change syncAllowance default value as an environment variable #158

Merged
merged 3 commits into from
Jul 13, 2021
Merged

Change syncAllowance default value as an environment variable #158

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8860131
move syncAllowance var to env const
rem1niscence Jul 13, 2021
bddc518
change null coalescing operator from env variables
rem1niscence Jul 13, 2021
0a58339
fix: change syncAllowance to be injected through loopback
rem1niscence Jul 13, 2021
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
2 changes: 2 additions & 0 deletions .env.example
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ POCKET_SESSION_BLOCK_FREQUENCY=4
POCKET_BLOCK_TIME=1038000
POCKET_RELAY_RETRIES=0

DEFAULT_SYNC_ALLOWANCE = 5

# Choose your AAT plan
# values: freemium, premium
AAT_PLAN=freemium
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/production-ap-southeast-1.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ jobs:
envkey_POCKET_SESSION_BLOCK_FREQUENCY: 4
envkey_POCKET_BLOCK_TIME: 1038000
envkey_POCKET_RELAY_RETRIES: 0
envkey_DEFAULT_SYNC_ALLOWANCE: 5
envkey_AAT_PLAN: 'premium'
envkey_NODE_ENV: 'production'
file_name: .env
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/production-eu-west-1.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ jobs:
envkey_POCKET_SESSION_BLOCK_FREQUENCY: 4
envkey_POCKET_BLOCK_TIME: 1038000
envkey_POCKET_RELAY_RETRIES: 0
envkey_DEFAULT_SYNC_ALLOWANCE: 5
envkey_AAT_PLAN: 'premium'
envkey_NODE_ENV: 'production'
file_name: .env
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/production-us-east-2.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ jobs:
envkey_POCKET_SESSION_BLOCK_FREQUENCY: 4
envkey_POCKET_BLOCK_TIME: 1038000
envkey_POCKET_RELAY_RETRIES: 0
envkey_DEFAULT_SYNC_ALLOWANCE: 5
envkey_AAT_PLAN: 'premium'
envkey_NODE_ENV: 'production'
file_name: .env
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/production-us-west-2.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ jobs:
envkey_POCKET_SESSION_BLOCK_FREQUENCY: 4
envkey_POCKET_BLOCK_TIME: 1038000
envkey_POCKET_RELAY_RETRIES: 0
envkey_DEFAULT_SYNC_ALLOWANCE: 5
envkey_AAT_PLAN: 'premium'
envkey_NODE_ENV: 'production'
file_name: .env
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/staging-us-west-2.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ jobs:
envkey_POCKET_SESSION_BLOCK_FREQUENCY: 4
envkey_POCKET_BLOCK_TIME: 1038000
envkey_POCKET_RELAY_RETRIES: 0
envkey_DEFAULT_SYNC_ALLOWANCE: 5
envkey_AAT_PLAN: 'premium'
envkey_NODE_ENV: 'production'
file_name: .env
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/testnet.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ jobs:
envkey_POCKET_SESSION_BLOCK_FREQUENCY: 4
envkey_POCKET_BLOCK_TIME: 900000
envkey_POCKET_RELAY_RETRIES: 5
envkey_DEFAULT_SYNC_ALLOWANCE: 5
envkey_AAT_PLAN: 'premium'
file_name: .env

Expand Down
24 changes: 12 additions & 12 deletions src/application.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,19 +44,19 @@ export class PocketGatewayApplication extends BootMixin(ServiceMixin(RepositoryM
// Requirements; for Production these are stored in GitHub repo secrets
//
// For Dev, you need to pass them in via .env file
const environment: string = process.env.NODE_ENV ?? 'production'
const environment: string = process.env.NODE_ENV || 'production'

logger.log('info', 'Environment: ' + environment)

const dispatchURL: string = process.env.DISPATCH_URL ?? ''
const altruists: string = process.env.ALTRUISTS ?? ''
const clientPrivateKey: string = process.env.GATEWAY_CLIENT_PRIVATE_KEY ?? ''
const clientPassphrase: string = process.env.GATEWAY_CLIENT_PASSPHRASE ?? ''
const pocketSessionBlockFrequency: string = process.env.POCKET_SESSION_BLOCK_FREQUENCY ?? ''
const pocketBlockTime: string = process.env.POCKET_BLOCK_TIME ?? ''
const relayRetries: string = process.env.POCKET_RELAY_RETRIES ?? ''
const databaseEncryptionKey: string = process.env.DATABASE_ENCRYPTION_KEY ?? ''
const aatPlan = process.env.AAT_PLAN ?? AatPlans.PREMIUM
const dispatchURL: string = process.env.DISPATCH_URL || ''
const altruists: string = process.env.ALTRUISTS || ''
const clientPrivateKey: string = process.env.GATEWAY_CLIENT_PRIVATE_KEY || ''
const clientPassphrase: string = process.env.GATEWAY_CLIENT_PASSPHRASE || ''
const pocketSessionBlockFrequency: string = process.env.POCKET_SESSION_BLOCK_FREQUENCY || ''
const pocketBlockTime: string = process.env.POCKET_BLOCK_TIME || ''
const relayRetries: string = process.env.POCKET_RELAY_RETRIES || ''
const databaseEncryptionKey: string = process.env.DATABASE_ENCRYPTION_KEY || ''
const aatPlan = process.env.AAT_PLAN || AatPlans.PREMIUM

if (!dispatchURL) {
throw new HttpErrors.InternalServerError('DISPATCH_URL required in ENV')
Expand Down Expand Up @@ -144,8 +144,8 @@ export class PocketGatewayApplication extends BootMixin(ServiceMixin(RepositoryM
this.bind('redisInstance').to(redis)

// Load Postgres for TimescaleDB metrics
const pgConnection: string = process.env.PG_CONNECTION ?? ''
const pgCertificate: string = process.env.PG_CERTIFICATE ?? ''
const pgConnection: string = process.env.PG_CONNECTION || ''
const pgCertificate: string = process.env.PG_CERTIFICATE || ''

if (!pgConnection) {
throw new HttpErrors.InternalServerError('PG_CONNECTION required in ENV')
Expand Down
4 changes: 2 additions & 2 deletions src/services/logger.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ interface Log {
elapsedTime: number
}

const environment: string = process.env.NODE_ENV ?? 'production'
const logzToken: string = process.env.LOGZ_TOKEN ?? ''
const environment: string = process.env.NODE_ENV || 'production'
const logzToken: string = process.env.LOGZ_TOKEN || ''

if (!logzToken && environment === 'production') {
throw new HttpErrors.InternalServerError('LOGZ_TOKEN required in ENV')
Expand Down
6 changes: 3 additions & 3 deletions src/services/sync-checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ const logger = require('../services/logger')

import axios from 'axios'

const DEFAULT_SYNC_ALLOWANCE: number = parseInt(process.env.DEFAULT_SYNC_ALLOWANCE) || 5
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather have all of the .env variables checked in one place and rather than silently fallback to a default, I'd rather it error out and tell you that you made a dumb mistake and forgot the var.

Probably around here:

portal-api/src/application.ts

Line 59 in bddc518

const aatPlan = process.env.AAT_PLAN || AatPlans.PREMIUM

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like having .env vars in one place—this is how I usually do it

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That does look nicer.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep, like it too

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for now I'm doing a quick fix but an issue can be opened for that to change all the envs


export class SyncChecker {
redis: Redis
metricsRecorder: MetricsRecorder
Expand All @@ -32,9 +34,7 @@ export class SyncChecker {
pocketConfiguration,
}: ConsensusFilterOptions): Promise<Node[]> {
// Blockchain records passed in with 0 sync allowance are missing the 'syncAllowance' field in MongoDB
if (syncAllowance <= 0) {
syncAllowance = 5
}
syncAllowance = syncAllowance <= 0 ? syncAllowance : DEFAULT_SYNC_ALLOWANCE

const syncedNodes: Node[] = []
let syncedNodesList: string[] = []
Expand Down