UC Volume ingestion: uniform behavior on SQL REMOVE (#249)

Signed-off-by: Levko Kravets <[email protected]>

lib/DBSQLSession.ts

@@ -287,11 +287,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}`);
     }
   }

tests/e2e/staging_ingestion.test.js

@@ -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] });
   });
 });