-
Notifications
You must be signed in to change notification settings - Fork 484
feat(api): add NANGO_ADMIN_PROD_ENV env var #3828
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: master
Are you sure you want to change the base?
Conversation
@@ -30,7 +32,7 @@ class ConnectionController { | |||
|
|||
const integration_key = process.env['NANGO_SLACK_INTEGRATION_KEY'] || 'slack'; | |||
const nangoAdminUUID = NANGO_ADMIN_UUID; | |||
const env = 'prod'; | |||
const env = process.env['NANGO_ADMIN_PROD_ENV'] || 'prod'; |
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.
we now prefer to use the validated and type safed parsedEnvs
. Example:
nango/packages/server/lib/server.ts
Line 33 in 5ea7299
initSentry({ dsn: envs.SENTRY_DSN, applicationName: envs.NANGO_DB_APPLICATION_NAME, hash: envs.GIT_HASH }); |
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.
yes, and that way you can put a default value too
@@ -85,7 +87,7 @@ export class SlackService { | |||
|
|||
private integrationKey = process.env['NANGO_SLACK_INTEGRATION_KEY'] || 'slack'; | |||
private nangoAdminUUID = process.env['NANGO_ADMIN_UUID']; | |||
private env = 'prod'; | |||
private env = process.env['NANGO_ADMIN_PROD_ENV'] || 'prod'; |
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.
nb in shared you wont be able to use parsed, but you can refactor the class to accept the env in the constructor
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.
Looks good, just changing to envs and it will be fine
The environment name will soon be editable, so I'm making this value editable via env var to make it clearer.
Part of NAN-2685