-
Notifications
You must be signed in to change notification settings - Fork 1k
fix(sdk): send the upload-request headers the API returns for template file uploads #1876
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
Closed
tomassrnka
wants to merge
2
commits into
chore/remove-template-build-v1
from
fix/template-upload-headers
Closed
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| --- | ||
| 'e2b': patch | ||
| '@e2b/python-sdk': patch | ||
| --- | ||
|
|
||
| Template file uploads now send the request headers the file-upload-link response returns. Azure-backed clusters sign layer-file uploads with a SAS and return `x-ms-blob-type: BlockBlob`, which a SAS cannot carry; without it every uncached `COPY` in `Template.build()` failed with a storage `400 MissingRequiredHeader`. GCS and S3 clusters return no headers and are unaffected. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,118 @@ | ||
| import { afterAll, afterEach, beforeAll, expect, test } from 'vitest' | ||
| import { writeFile, mkdtemp, rm } from 'fs/promises' | ||
| import { join } from 'path' | ||
| import { tmpdir } from 'os' | ||
| import { createServer, type IncomingMessage, type Server } from 'http' | ||
| import { AddressInfo } from 'net' | ||
| import { http, HttpResponse } from 'msw' | ||
| import { setupServer } from 'msw/node' | ||
| import { randomUUID } from 'node:crypto' | ||
|
|
||
| import { Template } from '../../src' | ||
| import { TEST_API_KEY, apiUrl } from '../setup' | ||
|
|
||
| // The file-upload-link response carries headers the signed URL cannot carry | ||
| // itself (Azure's Put Blob requires x-ms-blob-type); without them every | ||
| // uncached COPY fails with a storage 400 on an Azure-backed cluster. | ||
|
|
||
| let testDir: string | ||
| let uploadServer: Server | ||
| let uploadUrl: string | ||
| let capturedHeaders: IncomingMessage['headers'] = {} | ||
|
|
||
| let linkHeaders: Record<string, string> | undefined | ||
|
|
||
| const restHandlers = [ | ||
| http.post(apiUrl('/v3/templates'), async ({ request }) => { | ||
| const { name } = (await request.clone().json()) as { name: string } | ||
| return HttpResponse.json({ | ||
| buildID: randomUUID(), | ||
| templateID: name, | ||
| tags: [], | ||
| }) | ||
| }), | ||
| http.get(apiUrl('/templates/:templateID/files/:hash'), () => | ||
| HttpResponse.json({ | ||
| present: false, | ||
| url: uploadUrl, | ||
| headers: linkHeaders, | ||
| }) | ||
| ), | ||
| http.post(apiUrl('/v2/templates/:templateID/builds/:buildID'), () => | ||
| HttpResponse.json({}) | ||
| ), | ||
| ] | ||
|
|
||
| const server = setupServer(...restHandlers) | ||
|
|
||
| beforeAll(async () => { | ||
| testDir = await mkdtemp(join(tmpdir(), 'uploadHeaders-test-')) | ||
| await writeFile(join(testDir, 'hello.txt'), 'hello world') | ||
|
|
||
| uploadServer = createServer((req, res) => { | ||
| capturedHeaders = req.headers | ||
| req.on('data', () => {}) | ||
| req.on('end', () => { | ||
| res.writeHead(200) | ||
| res.end() | ||
| }) | ||
| }) | ||
| await new Promise<void>((resolve) => | ||
| uploadServer.listen(0, '127.0.0.1', resolve) | ||
| ) | ||
| const { port } = uploadServer.address() as AddressInfo | ||
| uploadUrl = `http://127.0.0.1:${port}/upload` | ||
|
|
||
| // Only the local upload server may go unmocked. print.error() alone still | ||
| // performs the request, so anything else is blocked by throwing. | ||
| server.listen({ | ||
| onUnhandledRequest: (request, print) => { | ||
| if (new URL(request.url).hostname === '127.0.0.1') return | ||
| print.error() | ||
| throw new Error(`unhandled request: ${request.method} ${request.url}`) | ||
| }, | ||
| }) | ||
| }) | ||
|
|
||
| afterAll(async () => { | ||
| server.close() | ||
| await new Promise<void>((resolve) => uploadServer.close(() => resolve())) | ||
| await rm(testDir, { recursive: true, force: true }) | ||
| }) | ||
|
|
||
| afterEach(() => { | ||
| capturedHeaders = {} | ||
| linkHeaders = undefined | ||
| }) | ||
|
|
||
| test('upload PUT carries the headers the upload link returned', async () => { | ||
| linkHeaders = { 'x-ms-blob-type': 'BlockBlob' } | ||
|
|
||
| const template = Template({ fileContextPath: testDir }) | ||
| .fromBaseImage() | ||
| .copy('*.txt', '.') | ||
|
|
||
| await Template.buildInBackground(template, 'upload-headers', { | ||
| apiKey: TEST_API_KEY, | ||
| }) | ||
|
|
||
| expect(capturedHeaders['x-ms-blob-type']).toBe('BlockBlob') | ||
| expect(Number(capturedHeaders['content-length'])).toBeGreaterThan(0) | ||
| expect( | ||
| (capturedHeaders['transfer-encoding'] ?? '').toLowerCase() | ||
| ).not.toContain('chunked') | ||
| expect(capturedHeaders['content-type']).toBeUndefined() | ||
| }) | ||
|
|
||
| test('upload PUT is unchanged when the upload link returns no headers', async () => { | ||
| const template = Template({ fileContextPath: testDir }) | ||
| .fromBaseImage() | ||
| .copy('*.txt', '.') | ||
|
|
||
| await Template.buildInBackground(template, 'upload-no-headers', { | ||
| apiKey: TEST_API_KEY, | ||
| }) | ||
|
|
||
| expect(capturedHeaders['x-ms-blob-type']).toBeUndefined() | ||
| expect(Number(capturedHeaders['content-length'])).toBeGreaterThan(0) | ||
| }) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back 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.
T-3a — new optional parameters are keyword-only from day one, enforced by a bare
*. As writtenheadersis a defaulted positional inserted ahead ofrequest_timeout, so any caller bindingrequest_timeoutby position now silently passes it asheaders. Internal today, but the rule exists to make that impossible; all call sites already use kwargs so adding*is safe.