-
Notifications
You must be signed in to change notification settings - Fork 77
feat: populate Blobs context in build plugins #5571
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
Conversation
@@ -26,12 +26,12 @@ const setTimeoutPromise = promisify(setTimeout) | |||
// response: json payload response (defaults to {}) | |||
// status: http status code (defaults to 200) | |||
// wait: number used to induce a certain time delay in milliseconds in the response (defaults to undefined) | |||
export const startServer = async (handler: ServerHandler) => { | |||
export const startServer = async (handler: ServerHandler, port = 0) => { |
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.
Sometimes it's useful to know the port before starting the server, so this makes it possible for the consumer to specify the port that the server should use.
This pull request adds or modifies JavaScript ( |
token?: string | ||
} | ||
|
||
// TODO: Move this work to a method exported by `@netlify/blobs`. |
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.
Something I've done in the past is attach a ticket number to TODO comments. This helps ensures that the work gets finished, gives a space to provide more detailed context, and makes it easier for other people to pick up the work.
[ | ||
{ | ||
response: { url: `http://localhost:${serverPort}/some-signed-url` }, | ||
path: `/api/v1/blobs/${siteId}/deploy:${deployId}/my-key`, |
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.
Maybe we should we have a test with region included as well?
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.
Some small and optional fixes, but overall looks good to me
@@ -598,73 +598,6 @@ Generated by [AVA](https://avajs.dev). | |||
(Netlify Build completed in 1ms)␊ | |||
Build step duration: Netlify Build completed in 1ms` | |||
|
|||
## Functions: install dependencies handles errors |
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 was getting a weird test failure in MacOS, so I got rid of the snapshot.
@@ -846,68 +779,6 @@ Generated by [AVA](https://avajs.dev). | |||
(Netlify Build completed in 1ms)␊ | |||
Build step duration: Netlify Build completed in 1ms` | |||
|
|||
## Install local plugin dependencies: propagate errors |
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 was getting a weird test failure in MacOS, so I got rid of the snapshot.
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.
Suffering same issue, thanks for this
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.
@lemusthelroy can I get a ✅? I lost Simon's when I updated the tests.
Summary
Populates the Blobs context in the build plugins execution environment, so that build plugins can interact with Blobs without having to initialise the client with access credentials and API addresses. It makes the experience consistent with the functions and edge functions entry points.
Closes COM-542.