Skip to content

Commit 1255fad

Browse files
UC Volume ingestion: uniform behavior on SQL REMOVE (#249)
Signed-off-by: Levko Kravets <levko.ne@gmail.com>
1 parent c52f2ba commit 1255fad

File tree

2 files changed

+18
-7
lines changed

2 files changed

+18
-7
lines changed

lib/DBSQLSession.ts

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -287,11 +287,14 @@ export default class DBSQLSession implements IDBSQLSession {
287287
const agent = await connectionProvider.getAgent();
288288

289289
const response = await fetch(presignedUrl, { method: 'DELETE', headers, agent });
290-
// Looks that AWS and Azure have a different behavior of HTTP `DELETE` for non-existing files
291-
// AWS assumes that - since file already doesn't exist - the goal is achieved, and returns HTTP 200
290+
// Looks that AWS and Azure have a different behavior of HTTP `DELETE` for non-existing files.
291+
// AWS assumes that - since file already doesn't exist - the goal is achieved, and returns HTTP 200.
292292
// Azure, on the other hand, is somewhat stricter and check if file exists before deleting it. And if
293-
// file doesn't exist - Azure returns HTTP 404
294-
if (!response.ok) {
293+
// file doesn't exist - Azure returns HTTP 404.
294+
//
295+
// For us, it's totally okay if file didn't exist before removing. So when we get an HTTP 404 -
296+
// just ignore it and report success. This way we can have a uniform library behavior for all clouds
297+
if (!response.ok && response.status !== 404) {
295298
throw new StagingError(`HTTP error ${response.status} ${response.statusText}`);
296299
}
297300
}

tests/e2e/staging_ingestion.test.js

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -114,17 +114,25 @@ describe('Staging Test', () => {
114114
});
115115

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

119+
// File should not exist before removing
118120
try {
119-
await session.executeStatement(`REMOVE '${stagingFileName}'`, { stagingAllowedLocalPath: [localPath] });
120-
// In some cases, `REMOVE` may silently succeed for non-existing files (see comment in relevant
121-
// part of `DBSQLSession` code). But if it fails - it has to be an HTTP 404 error
121+
await session.executeStatement(`GET '${stagingFileName}' TO '${localFile}'`, {
122+
stagingAllowedLocalPath: [localPath],
123+
});
124+
expect.fail('It should throw HTTP 404 error');
122125
} catch (error) {
123126
if (error instanceof StagingError) {
124127
expect(error.message).to.contain('404');
125128
} else {
126129
throw error;
127130
}
131+
} finally {
132+
fs.rmSync(localFile, { force: true });
128133
}
134+
135+
// Try to remove the file - it should succeed and not throw any errors
136+
await session.executeStatement(`REMOVE '${stagingFileName}'`, { stagingAllowedLocalPath: [localPath] });
129137
});
130138
});

0 commit comments

Comments
 (0)