Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions lib/DBSQLSession.ts
Original file line number Diff line number Diff line change
Expand Up @@ -280,11 +280,14 @@ export default class DBSQLSession implements IDBSQLSession {
const agent = await connectionProvider.getAgent();

const response = await fetch(presignedUrl, { method: 'DELETE', headers, agent });
// Looks that AWS and Azure have a different behavior of HTTP `DELETE` for non-existing files
// AWS assumes that - since file already doesn't exist - the goal is achieved, and returns HTTP 200
// Looks that AWS and Azure have a different behavior of HTTP `DELETE` for non-existing files.
// AWS assumes that - since file already doesn't exist - the goal is achieved, and returns HTTP 200.
// Azure, on the other hand, is somewhat stricter and check if file exists before deleting it. And if
// file doesn't exist - Azure returns HTTP 404
if (!response.ok) {
// file doesn't exist - Azure returns HTTP 404.
//
// For us, it's totally okay if file didn't exist before removing. So when we get an HTTP 404 -
// just ignore it and report success. This way we can have a uniform library behavior for all clouds
if (!response.ok && response.status !== 404) {
throw new StagingError(`HTTP error ${response.status} ${response.statusText}`);
}
}
Expand Down
14 changes: 11 additions & 3 deletions tests/e2e/staging_ingestion.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -114,17 +114,25 @@ describe('Staging Test', () => {
});

const stagingFileName = `/Volumes/${catalog}/${schema}/${volume}/${uuid.v4()}.csv`;
const localFile = path.join(localPath, `${uuid.v4()}.csv`);

// File should not exist before removing
try {
await session.executeStatement(`REMOVE '${stagingFileName}'`, { stagingAllowedLocalPath: [localPath] });
// In some cases, `REMOVE` may silently succeed for non-existing files (see comment in relevant
// part of `DBSQLSession` code). But if it fails - it has to be an HTTP 404 error
await session.executeStatement(`GET '${stagingFileName}' TO '${localFile}'`, {
stagingAllowedLocalPath: [localPath],
});
expect.fail('It should throw HTTP 404 error');
} catch (error) {
if (error instanceof StagingError) {
expect(error.message).to.contain('404');
} else {
throw error;
}
} finally {
fs.rmSync(localFile, { force: true });
}

// Try to remove the file - it should succeed and not throw any errors
await session.executeStatement(`REMOVE '${stagingFileName}'`, { stagingAllowedLocalPath: [localPath] });
});
});