-
Notifications
You must be signed in to change notification settings - Fork 455
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
[NAN-676] enforce cap when activating a script #1953
[NAN-676] enforce cap when activating a script #1953
Conversation
…ers' into khaliq/nan-677-enforce-cap-when-calling-nangoauth
…ers' into khaliq/nan-677-enforce-cap-when-calling-nangoauth
…rect values via the API
…-enforce-cap-when-calling-nangoauth
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.
question about the comment out limit constant but looks good otherwise
@@ -1,2 +1,4 @@ | |||
export const SYNC_TASK_QUEUE = 'nango-syncs'; | |||
export const WEBHOOK_TASK_QUEUE = 'nango-webhooks'; | |||
//export const CONNECTIONS_WITH_SCRIPTS_CAP_LIMIT = 3; | |||
export const CONNECTIONS_WITH_SCRIPTS_CAP_LIMIT = Infinity; |
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.
is that on purpose?
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, very much so. We don't want to cap until we can set the account information correctly via the API
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.
Just thought about it during the night (could not sleep). We also have to think about enterprise/community 👌🏻
setModalOkButtonTitle('Upgrade'); | ||
setModalCancelButtonTitle('Learn more'); | ||
setModalOkButtonLink('https://nango.dev/chat'); | ||
setModalCancelButtonLink('https://docs.nango.dev/REPLACE-ME'); |
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.
is it going to be replace later?
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.
Yup, launch blocker to replace.
@@ -990,6 +991,25 @@ class ConnectionService { | |||
} | |||
} | |||
|
|||
public async shouldCapUsage({ providerConfigKey, environmentId }: { providerConfigKey: string | undefined; environmentId: number }): Promise<boolean> { |
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.
Can it really be undefined?
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.
I'll have to look again at it. It probably shouldn't be
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.
Thanks, updated.
Describe your changes
Builds on #1950
Issue ticket number and link
NAN-676
Checklist before requesting a review (skip if just adding/editing APIs & templates)