-
Notifications
You must be signed in to change notification settings - Fork 374
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
feat: custom rate limit rules payload #6503
Conversation
29f5861
to
84933cc
Compare
84933cc
to
0ceb7f8
Compare
src/utils/deploy/hash-fns.ts
Outdated
// @ts-expect-error TS(7031) FIXME: Binding element 'buildData' implicitly has an 'any... Remove this comment to see the full error message | ||
({ buildData, displayName, generator, invocationMode, path: functionPath, priority, runtime, runtimeVersion }) => ({ | ||
({ | ||
// @ts-expect-error TS(7031) FIXME: Binding element 'buildData' implicitly has an 'any... Remove this comment to see the full error message |
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 someone knows a way to apply this to the whole block, please do share 😅
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.
the fix is to write some proper types for this :D Let me see what I can do.
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 in 8f4a538!
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.
woah, thank you so much!
src/utils/deploy/hash-fns.ts
Outdated
@@ -195,4 +219,25 @@ const hashFns = async ( | |||
return { functionSchedules, functions, functionsWithNativeModules, fnShaMap, fnConfig } | |||
} | |||
|
|||
const trafficRulesConfig = (trafficRules: $TSFixMe) => { |
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 would be nice to import the type that we'll expose to users and use it here, but we can do that as a follow-up.
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.
in 8f4a538, i've imported the TrafficRules type from the ZISI internals. Is that what you had in mind? Also, do we consider importing those internals criminal?
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, exactly! I don't think it's criminal, but I think it would be good to properly export them so we're not relying on internal file paths.
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.
agreed. i'll make the zisi PR and replace the import in a followup.
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.
here's said PR: netlify/zip-it-and-ship-it#1724
export const config = { path: "/internal-v2-func" } | ||
export const config = { | ||
path: "/internal-v2-func", | ||
rateLimit: { |
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 don't see any assertions added. I'm assuming you used this for testing things locally? Fine by me, i'm wondering if there's a good way of asserting that the ratelimit config was actually applied 🤔
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 I tested it out locally
Summary
Related to ADN-294, sends the same payload to the API we're sending now on normal builds.