From 86bc8de6efa01b7d75dcee2698f48cdfefa147c2 Mon Sep 17 00:00:00 2001 From: Miguel Branco Date: Tue, 13 Aug 2024 10:00:15 +0200 Subject: [PATCH 1/6] Add ErrorConfig to hold errors contacting locations. --- protocol/src/main/protobuf/raw/protocol/protocol.proto | 5 +++++ .../scala/raw/compiler/rql2/api/LocationDescription.scala | 2 ++ .../main/scala/raw/compiler/rql2/builtin/MySQLPackage.scala | 2 ++ .../scala/raw/compiler/rql2/builtin/OraclePackage.scala | 2 ++ .../scala/raw/compiler/rql2/builtin/PostgreSQLPackage.scala | 2 ++ .../scala/raw/compiler/rql2/builtin/SQLServerPackage.scala | 2 ++ .../scala/raw/compiler/rql2/builtin/SnowflakePackage.scala | 2 ++ .../src/main/java/raw/runtime/truffle/RawContext.java | 6 +++++- 8 files changed, 22 insertions(+), 1 deletion(-) diff --git a/protocol/src/main/protobuf/raw/protocol/protocol.proto b/protocol/src/main/protobuf/raw/protocol/protocol.proto index dfe089835..486a7ea2e 100644 --- a/protocol/src/main/protobuf/raw/protocol/protocol.proto +++ b/protocol/src/main/protobuf/raw/protocol/protocol.proto @@ -23,6 +23,7 @@ message LocationConfig { DropboxUsernamePasswordConfig dropboxUsernamePassword = 14; HttpHeadersConfig httpHeaders = 15; SecretConfig secret = 99; + ErrorConfig error = 9999; } } @@ -145,4 +146,8 @@ message HttpHeadersConfig { message SecretConfig { string name = 1; string value = 2; +} + +message ErrorConfig { + string message = 1; } \ No newline at end of file diff --git a/snapi-frontend/src/main/scala/raw/compiler/rql2/api/LocationDescription.scala b/snapi-frontend/src/main/scala/raw/compiler/rql2/api/LocationDescription.scala index 979ed1fb5..baf3df2fc 100644 --- a/snapi-frontend/src/main/scala/raw/compiler/rql2/api/LocationDescription.scala +++ b/snapi-frontend/src/main/scala/raw/compiler/rql2/api/LocationDescription.scala @@ -624,6 +624,7 @@ object LocationDescription extends StrictLogging { objectKey ) ) + case Some(l) if l.hasError => Left(l.getError.getMessage) case None => // Anonymous access. Right(S3PathLocationDescription(bucketName, None, None, None, objectKey)) @@ -651,6 +652,7 @@ object LocationDescription extends StrictLogging { path ) ) + case Some(l) if l.hasError => Left(l.getError.getMessage) case None => Left("missing Dropbox credential") } case _ => Left(s"unsupported protocol: $protocol") diff --git a/snapi-frontend/src/main/scala/raw/compiler/rql2/builtin/MySQLPackage.scala b/snapi-frontend/src/main/scala/raw/compiler/rql2/builtin/MySQLPackage.scala index 198a02b9b..e8afbbc36 100644 --- a/snapi-frontend/src/main/scala/raw/compiler/rql2/builtin/MySQLPackage.scala +++ b/snapi-frontend/src/main/scala/raw/compiler/rql2/builtin/MySQLPackage.scala @@ -150,6 +150,7 @@ class MySQLInferAndReadEntry extends SugarEntryExtension { new MySqlTableLocation(l1.getHost, l1.getPort, l1.getDatabase, l1.getUser, l1.getPassword, table)( programContext.settings ) + case Some(l) if l.hasError => Left(l.getError.getMessage) case Some(_) => return Left("not a MySQL server") case None => return Left(s"unknown credential: $db") } @@ -394,6 +395,7 @@ class MySQLInferAndQueryEntry extends SugarEntryExtension { new MySqlServerLocation(l1.getHost, l1.getPort, l1.getDatabase, l1.getUser, l1.getPassword)( programContext.settings ) + case Some(l) if l.hasError => Left(l.getError.getMessage) case Some(_) => return Left("not a MySQL server") case None => return Left(s"unknown credential: $db") } diff --git a/snapi-frontend/src/main/scala/raw/compiler/rql2/builtin/OraclePackage.scala b/snapi-frontend/src/main/scala/raw/compiler/rql2/builtin/OraclePackage.scala index 5475fc3cd..cafaf081e 100644 --- a/snapi-frontend/src/main/scala/raw/compiler/rql2/builtin/OraclePackage.scala +++ b/snapi-frontend/src/main/scala/raw/compiler/rql2/builtin/OraclePackage.scala @@ -170,6 +170,7 @@ class OracleInferAndReadEntry extends SugarEntryExtension { new OracleTableLocation(l1.getHost, l1.getPort, l1.getDatabase, l1.getUser, l1.getPassword, schema, table)( programContext.settings ) + case Some(l) if l.hasError => Left(l.getError.getMessage) case Some(_) => return Left("not an Oracle server") case None => return Left(s"unknown credential: $db") } @@ -423,6 +424,7 @@ class OracleInferAndQueryEntry extends SugarEntryExtension { new OracleServerLocation(l1.getHost, l1.getPort, l1.getDatabase, l1.getUser, l1.getPassword)( programContext.settings ) + case Some(l) if l.hasError => Left(l.getError.getMessage) case Some(_) => return Left("not an Oracle server") case None => return Left(s"unknown credential: $db") } diff --git a/snapi-frontend/src/main/scala/raw/compiler/rql2/builtin/PostgreSQLPackage.scala b/snapi-frontend/src/main/scala/raw/compiler/rql2/builtin/PostgreSQLPackage.scala index 3770f92ef..81f6cd9e0 100644 --- a/snapi-frontend/src/main/scala/raw/compiler/rql2/builtin/PostgreSQLPackage.scala +++ b/snapi-frontend/src/main/scala/raw/compiler/rql2/builtin/PostgreSQLPackage.scala @@ -178,6 +178,7 @@ class PostgreSQLInferAndReadEntry extends SugarEntryExtension { )( programContext.settings ) + case Some(l) if l.hasError => Left(l.getError.getMessage) case Some(_) => return Left("not a PostgreSQL server") case None => return Left(s"unknown credential: $db") } @@ -433,6 +434,7 @@ class PostgreSQLInferAndQueryEntry extends SugarEntryExtension { new PostgresqlServerLocation(l1.getHost, l1.getPort, l1.getDatabase, l1.getUser, l1.getPassword)( programContext.settings ) + case Some(l) if l.hasError => Left(l.getError.getMessage) case Some(_) => return Left("not an Oracle server") case None => return Left(s"unknown credential: $db") } diff --git a/snapi-frontend/src/main/scala/raw/compiler/rql2/builtin/SQLServerPackage.scala b/snapi-frontend/src/main/scala/raw/compiler/rql2/builtin/SQLServerPackage.scala index 1f57e1aab..0db94f0e9 100644 --- a/snapi-frontend/src/main/scala/raw/compiler/rql2/builtin/SQLServerPackage.scala +++ b/snapi-frontend/src/main/scala/raw/compiler/rql2/builtin/SQLServerPackage.scala @@ -179,6 +179,7 @@ class SQLServerInferAndReadEntry extends SugarEntryExtension { )( programContext.settings ) + case Some(l) if l.hasError => Left(l.getError.getMessage) case Some(_) => return Left("not an Oracle server") case None => return Left(s"unknown credential: $db") } @@ -435,6 +436,7 @@ class SQLServerInferAndQueryEntry extends SugarEntryExtension { new SqlServerServerLocation(l1.getHost, l1.getPort, l1.getDatabase, l1.getUser, l1.getPassword)( programContext.settings ) + case Some(l) if l.hasError => Left(l.getError.getMessage) case Some(_) => return Left("not an Oracle server") case None => return Left(s"unknown credential: $db") } diff --git a/snapi-frontend/src/main/scala/raw/compiler/rql2/builtin/SnowflakePackage.scala b/snapi-frontend/src/main/scala/raw/compiler/rql2/builtin/SnowflakePackage.scala index fcbe1be09..a7a2dc9f3 100644 --- a/snapi-frontend/src/main/scala/raw/compiler/rql2/builtin/SnowflakePackage.scala +++ b/snapi-frontend/src/main/scala/raw/compiler/rql2/builtin/SnowflakePackage.scala @@ -211,6 +211,7 @@ class SnowflakeInferAndReadEntry extends SugarEntryExtension { )( programContext.settings ) + case Some(l) if l.hasError => Left(l.getError.getMessage) case Some(_) => return Left("not a Snowflake server") case None => return Left(s"unknown credential: $db") } @@ -501,6 +502,7 @@ class SnowflakeInferAndQueryEntry extends SugarEntryExtension { l1.getParametersMap, programContext.settings ) + case Some(l) if l.hasError => Left(l.getError.getMessage) case Some(_) => return Left("not a Snowflake server") case None => return Left(s"unknown credential: $db") } diff --git a/snapi-truffle/src/main/java/raw/runtime/truffle/RawContext.java b/snapi-truffle/src/main/java/raw/runtime/truffle/RawContext.java index 3d7f2476b..703329409 100644 --- a/snapi-truffle/src/main/java/raw/runtime/truffle/RawContext.java +++ b/snapi-truffle/src/main/java/raw/runtime/truffle/RawContext.java @@ -123,7 +123,11 @@ public LocationConfig getLocationConfig(String name) { if (maybeLocationConfig.isEmpty()) { throw new RawTruffleRuntimeException("unknown credential: " + name); } - return maybeLocationConfig.get(); + LocationConfig locationConfig = maybeLocationConfig.get(); + if (locationConfig.hasError()) { + throw new RawTruffleRuntimeException(locationConfig.getError().getMessage()); + } + return locationConfig; } @CompilerDirectives.TruffleBoundary From af5ca6666b014895868ff281c9a41e7448c401f9 Mon Sep 17 00:00:00 2001 From: Miguel Branco Date: Tue, 13 Aug 2024 10:35:32 +0200 Subject: [PATCH 2/6] Fix to Dropbox. Fix to connection pooling. --- .../rql2/api/LocationDescription.scala | 16 +++++++ .../scala/raw/client/sql/SqlConnection.scala | 17 +++++--- .../raw/client/sql/SqlConnectionPool.scala | 43 ++++++++++--------- 3 files changed, 50 insertions(+), 26 deletions(-) diff --git a/snapi-frontend/src/main/scala/raw/compiler/rql2/api/LocationDescription.scala b/snapi-frontend/src/main/scala/raw/compiler/rql2/api/LocationDescription.scala index baf3df2fc..979548627 100644 --- a/snapi-frontend/src/main/scala/raw/compiler/rql2/api/LocationDescription.scala +++ b/snapi-frontend/src/main/scala/raw/compiler/rql2/api/LocationDescription.scala @@ -652,6 +652,22 @@ object LocationDescription extends StrictLogging { path ) ) + case Some(l) if l.hasHttpHeaders => + if (l.getHttpHeaders.getHeadersMap.containsKey("Authorization")) { + val splitted = l.getHttpHeaders.getHeadersMap.get("Authorization").split("Bearer ") + if (splitted.length == 2) { + Right( + DropboxAccessTokenLocationDescription( + splitted(1), + path + ) + ) + } else { + Left("invalid Dropbox credential") + } + } else { + Left("missing Dropbox credential") + } case Some(l) if l.hasError => Left(l.getError.getMessage) case None => Left("missing Dropbox credential") } diff --git a/sql-client/src/main/scala/raw/client/sql/SqlConnection.scala b/sql-client/src/main/scala/raw/client/sql/SqlConnection.scala index 8e0a5ae7b..7a13f2c87 100644 --- a/sql-client/src/main/scala/raw/client/sql/SqlConnection.scala +++ b/sql-client/src/main/scala/raw/client/sql/SqlConnection.scala @@ -33,11 +33,18 @@ import java.util.concurrent.Executor class SqlConnection(connectionPool: SqlConnectionPool, conn: Connection) extends java.sql.Connection { override def close(): Unit = { - // We do not ACTUALLY close the connection; instead, we just release the borrow. - connectionPool.releaseConnection( - this, - isAlive = false // We are not sure if the connection is alive or not, e.g. it could be closed because it failed. - ) + if (isClosed) { + // If the connection closed in the meantime (e.g. due to a crash), we cannot release it back to the pool. + connectionPool.actuallyRemoveConnection(this) + } else { + // If the connection seems "sane", then we do not ACTUALLY close the connection. + // Instead, we just release the borrow. + connectionPool.releaseConnection( + this, + isAlive = + false // We are not sure if the connection is alive or not, e.g. it could be closed because it failed. + ) + } } // This is called by the connection pool when we *actually* want to close the connection. diff --git a/sql-client/src/main/scala/raw/client/sql/SqlConnectionPool.scala b/sql-client/src/main/scala/raw/client/sql/SqlConnectionPool.scala index 794834048..07a29eb3d 100644 --- a/sql-client/src/main/scala/raw/client/sql/SqlConnectionPool.scala +++ b/sql-client/src/main/scala/raw/client/sql/SqlConnectionPool.scala @@ -90,7 +90,7 @@ class SqlConnectionPool()(implicit settings: RawSettings) extends RawService wit logger.debug(s"Checking the connection health for $conn (state: ${connectionUrls(conn)})") // Found one connection to check. try { - if (conn.isValid(isValidSeconds)) { + if (!conn.isClosed() && conn.isValid(isValidSeconds)) { logger.debug(s"Connection $conn is healthy") // All good, so release borrow. // This will update the last check is alive time. @@ -225,28 +225,29 @@ class SqlConnectionPool()(implicit settings: RawSettings) extends RawService wit } } - private def actuallyRemoveConnection(conn: SqlConnection): Boolean = { - logger.debug(s"Actually removing connection $conn") + def actuallyRemoveConnection(conn: SqlConnection): Boolean = { connectionPoolLock.synchronized { - val jdbcUrl = connectionUrls(conn) - try { - // First try to actually close the connection (note the use of actuallyClose), then clean up all the state. - conn.actuallyClose() - connectionState.remove(conn) - connectionUrls.remove(conn) - connectionCache.get(jdbcUrl) match { - case Some(conns) => - val nconns = conns - conn - if (nconns.isEmpty) connectionCache.remove(jdbcUrl) - else connectionCache.put(jdbcUrl, nconns) - case None => // Nothing to do. + connectionUrls.get(conn).foreach { jdbcUrl => + logger.debug(s"Actually removing connection $conn") + try { + // First try to actually close the connection (note the use of actuallyClose), then clean up all the state. + conn.actuallyClose() + connectionState.remove(conn) + connectionUrls.remove(conn) + connectionCache.get(jdbcUrl) match { + case Some(conns) => + val nconns = conns - conn + if (nconns.isEmpty) connectionCache.remove(jdbcUrl) + else connectionCache.put(jdbcUrl, nconns) + case None => // Nothing to do. + } + true + } catch { + case NonFatal(t) => + // We failed to actually close the connection. + logger.warn(s"Failed to actually close the connection $conn", t) + false } - true - } catch { - case NonFatal(t) => - // We failed to actually close the connection. - logger.warn(s"Failed to actually close the connection $conn", t) - false } } } From 7964247a01446c2fe6f85f165ad6f0daf0c0a27e Mon Sep 17 00:00:00 2001 From: Miguel Branco Date: Tue, 13 Aug 2024 10:42:06 +0200 Subject: [PATCH 3/6] Fix. --- .../raw/client/sql/SqlConnectionPool.scala | 44 ++++++++++--------- 1 file changed, 24 insertions(+), 20 deletions(-) diff --git a/sql-client/src/main/scala/raw/client/sql/SqlConnectionPool.scala b/sql-client/src/main/scala/raw/client/sql/SqlConnectionPool.scala index 07a29eb3d..0e0f9a392 100644 --- a/sql-client/src/main/scala/raw/client/sql/SqlConnectionPool.scala +++ b/sql-client/src/main/scala/raw/client/sql/SqlConnectionPool.scala @@ -227,27 +227,31 @@ class SqlConnectionPool()(implicit settings: RawSettings) extends RawService wit def actuallyRemoveConnection(conn: SqlConnection): Boolean = { connectionPoolLock.synchronized { - connectionUrls.get(conn).foreach { jdbcUrl => - logger.debug(s"Actually removing connection $conn") - try { - // First try to actually close the connection (note the use of actuallyClose), then clean up all the state. - conn.actuallyClose() - connectionState.remove(conn) - connectionUrls.remove(conn) - connectionCache.get(jdbcUrl) match { - case Some(conns) => - val nconns = conns - conn - if (nconns.isEmpty) connectionCache.remove(jdbcUrl) - else connectionCache.put(jdbcUrl, nconns) - case None => // Nothing to do. + connectionUrls.get(conn) match { + case Some(jdbcUrl) => + logger.debug(s"Actually removing connection $conn") + try { + // First try to actually close the connection (note the use of actuallyClose), then clean up all the state. + conn.actuallyClose() + connectionState.remove(conn) + connectionUrls.remove(conn) + connectionCache.get(jdbcUrl) match { + case Some(conns) => + val nconns = conns - conn + if (nconns.isEmpty) connectionCache.remove(jdbcUrl) + else connectionCache.put(jdbcUrl, nconns) + case None => // Nothing to do. + } + true + } catch { + case NonFatal(t) => + // We failed to actually close the connection. + logger.warn(s"Failed to actually close the connection $conn", t) + false } - true - } catch { - case NonFatal(t) => - // We failed to actually close the connection. - logger.warn(s"Failed to actually close the connection $conn", t) - false - } + case None => + // Connection didn't exist/was removed already. + false } } } From d2e757f461c6dc9e3e92638ba56f45c4d5d3a7c5 Mon Sep 17 00:00:00 2001 From: Miguel Branco Date: Tue, 13 Aug 2024 10:49:34 +0200 Subject: [PATCH 4/6] Fix. --- .../scala/raw/compiler/rql2/api/LocationDescription.scala | 5 +++-- .../main/scala/raw/compiler/rql2/builtin/MySQLPackage.scala | 4 ++-- .../main/scala/raw/compiler/rql2/builtin/OraclePackage.scala | 4 ++-- .../scala/raw/compiler/rql2/builtin/PostgreSQLPackage.scala | 4 ++-- .../scala/raw/compiler/rql2/builtin/SQLServerPackage.scala | 4 ++-- .../scala/raw/compiler/rql2/builtin/SnowflakePackage.scala | 4 ++-- 6 files changed, 13 insertions(+), 12 deletions(-) diff --git a/snapi-frontend/src/main/scala/raw/compiler/rql2/api/LocationDescription.scala b/snapi-frontend/src/main/scala/raw/compiler/rql2/api/LocationDescription.scala index 979548627..22052646a 100644 --- a/snapi-frontend/src/main/scala/raw/compiler/rql2/api/LocationDescription.scala +++ b/snapi-frontend/src/main/scala/raw/compiler/rql2/api/LocationDescription.scala @@ -624,7 +624,8 @@ object LocationDescription extends StrictLogging { objectKey ) ) - case Some(l) if l.hasError => Left(l.getError.getMessage) + case Some(l) if l.hasError => return Left(l.getError.getMessage) + case Some(_) => Left("missing S3 credential") case None => // Anonymous access. Right(S3PathLocationDescription(bucketName, None, None, None, objectKey)) @@ -668,7 +669,7 @@ object LocationDescription extends StrictLogging { } else { Left("missing Dropbox credential") } - case Some(l) if l.hasError => Left(l.getError.getMessage) + case Some(l) if l.hasError => return Left(l.getError.getMessage) case None => Left("missing Dropbox credential") } case _ => Left(s"unsupported protocol: $protocol") diff --git a/snapi-frontend/src/main/scala/raw/compiler/rql2/builtin/MySQLPackage.scala b/snapi-frontend/src/main/scala/raw/compiler/rql2/builtin/MySQLPackage.scala index e8afbbc36..33f85f03b 100644 --- a/snapi-frontend/src/main/scala/raw/compiler/rql2/builtin/MySQLPackage.scala +++ b/snapi-frontend/src/main/scala/raw/compiler/rql2/builtin/MySQLPackage.scala @@ -150,7 +150,7 @@ class MySQLInferAndReadEntry extends SugarEntryExtension { new MySqlTableLocation(l1.getHost, l1.getPort, l1.getDatabase, l1.getUser, l1.getPassword, table)( programContext.settings ) - case Some(l) if l.hasError => Left(l.getError.getMessage) + case Some(l) if l.hasError => return Left(l.getError.getMessage) case Some(_) => return Left("not a MySQL server") case None => return Left(s"unknown credential: $db") } @@ -395,7 +395,7 @@ class MySQLInferAndQueryEntry extends SugarEntryExtension { new MySqlServerLocation(l1.getHost, l1.getPort, l1.getDatabase, l1.getUser, l1.getPassword)( programContext.settings ) - case Some(l) if l.hasError => Left(l.getError.getMessage) + case Some(l) if l.hasError => return Left(l.getError.getMessage) case Some(_) => return Left("not a MySQL server") case None => return Left(s"unknown credential: $db") } diff --git a/snapi-frontend/src/main/scala/raw/compiler/rql2/builtin/OraclePackage.scala b/snapi-frontend/src/main/scala/raw/compiler/rql2/builtin/OraclePackage.scala index cafaf081e..d9273b185 100644 --- a/snapi-frontend/src/main/scala/raw/compiler/rql2/builtin/OraclePackage.scala +++ b/snapi-frontend/src/main/scala/raw/compiler/rql2/builtin/OraclePackage.scala @@ -170,7 +170,7 @@ class OracleInferAndReadEntry extends SugarEntryExtension { new OracleTableLocation(l1.getHost, l1.getPort, l1.getDatabase, l1.getUser, l1.getPassword, schema, table)( programContext.settings ) - case Some(l) if l.hasError => Left(l.getError.getMessage) + case Some(l) if l.hasError => return Left(l.getError.getMessage) case Some(_) => return Left("not an Oracle server") case None => return Left(s"unknown credential: $db") } @@ -424,7 +424,7 @@ class OracleInferAndQueryEntry extends SugarEntryExtension { new OracleServerLocation(l1.getHost, l1.getPort, l1.getDatabase, l1.getUser, l1.getPassword)( programContext.settings ) - case Some(l) if l.hasError => Left(l.getError.getMessage) + case Some(l) if l.hasError => return Left(l.getError.getMessage) case Some(_) => return Left("not an Oracle server") case None => return Left(s"unknown credential: $db") } diff --git a/snapi-frontend/src/main/scala/raw/compiler/rql2/builtin/PostgreSQLPackage.scala b/snapi-frontend/src/main/scala/raw/compiler/rql2/builtin/PostgreSQLPackage.scala index 81f6cd9e0..d75a82e00 100644 --- a/snapi-frontend/src/main/scala/raw/compiler/rql2/builtin/PostgreSQLPackage.scala +++ b/snapi-frontend/src/main/scala/raw/compiler/rql2/builtin/PostgreSQLPackage.scala @@ -178,7 +178,7 @@ class PostgreSQLInferAndReadEntry extends SugarEntryExtension { )( programContext.settings ) - case Some(l) if l.hasError => Left(l.getError.getMessage) + case Some(l) if l.hasError => return Left(l.getError.getMessage) case Some(_) => return Left("not a PostgreSQL server") case None => return Left(s"unknown credential: $db") } @@ -434,7 +434,7 @@ class PostgreSQLInferAndQueryEntry extends SugarEntryExtension { new PostgresqlServerLocation(l1.getHost, l1.getPort, l1.getDatabase, l1.getUser, l1.getPassword)( programContext.settings ) - case Some(l) if l.hasError => Left(l.getError.getMessage) + case Some(l) if l.hasError => return Left(l.getError.getMessage) case Some(_) => return Left("not an Oracle server") case None => return Left(s"unknown credential: $db") } diff --git a/snapi-frontend/src/main/scala/raw/compiler/rql2/builtin/SQLServerPackage.scala b/snapi-frontend/src/main/scala/raw/compiler/rql2/builtin/SQLServerPackage.scala index 0db94f0e9..5fa5a1230 100644 --- a/snapi-frontend/src/main/scala/raw/compiler/rql2/builtin/SQLServerPackage.scala +++ b/snapi-frontend/src/main/scala/raw/compiler/rql2/builtin/SQLServerPackage.scala @@ -179,7 +179,7 @@ class SQLServerInferAndReadEntry extends SugarEntryExtension { )( programContext.settings ) - case Some(l) if l.hasError => Left(l.getError.getMessage) + case Some(l) if l.hasError => return Left(l.getError.getMessage) case Some(_) => return Left("not an Oracle server") case None => return Left(s"unknown credential: $db") } @@ -436,7 +436,7 @@ class SQLServerInferAndQueryEntry extends SugarEntryExtension { new SqlServerServerLocation(l1.getHost, l1.getPort, l1.getDatabase, l1.getUser, l1.getPassword)( programContext.settings ) - case Some(l) if l.hasError => Left(l.getError.getMessage) + case Some(l) if l.hasError => return Left(l.getError.getMessage) case Some(_) => return Left("not an Oracle server") case None => return Left(s"unknown credential: $db") } diff --git a/snapi-frontend/src/main/scala/raw/compiler/rql2/builtin/SnowflakePackage.scala b/snapi-frontend/src/main/scala/raw/compiler/rql2/builtin/SnowflakePackage.scala index a7a2dc9f3..283ef0259 100644 --- a/snapi-frontend/src/main/scala/raw/compiler/rql2/builtin/SnowflakePackage.scala +++ b/snapi-frontend/src/main/scala/raw/compiler/rql2/builtin/SnowflakePackage.scala @@ -211,7 +211,7 @@ class SnowflakeInferAndReadEntry extends SugarEntryExtension { )( programContext.settings ) - case Some(l) if l.hasError => Left(l.getError.getMessage) + case Some(l) if l.hasError => return Left(l.getError.getMessage) case Some(_) => return Left("not a Snowflake server") case None => return Left(s"unknown credential: $db") } @@ -502,7 +502,7 @@ class SnowflakeInferAndQueryEntry extends SugarEntryExtension { l1.getParametersMap, programContext.settings ) - case Some(l) if l.hasError => Left(l.getError.getMessage) + case Some(l) if l.hasError => return Left(l.getError.getMessage) case Some(_) => return Left("not a Snowflake server") case None => return Left(s"unknown credential: $db") } From dd8f0698ecafa76e96a695edb947c9420d499ba3 Mon Sep 17 00:00:00 2001 From: Miguel Branco Date: Tue, 13 Aug 2024 10:51:13 +0200 Subject: [PATCH 5/6] Fix error messages. --- .../raw/compiler/rql2/api/LocationDescription.scala | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/snapi-frontend/src/main/scala/raw/compiler/rql2/api/LocationDescription.scala b/snapi-frontend/src/main/scala/raw/compiler/rql2/api/LocationDescription.scala index 22052646a..29aaa3d58 100644 --- a/snapi-frontend/src/main/scala/raw/compiler/rql2/api/LocationDescription.scala +++ b/snapi-frontend/src/main/scala/raw/compiler/rql2/api/LocationDescription.scala @@ -574,7 +574,7 @@ object LocationDescription extends StrictLogging { val delay = parser.getDuration("delay").toMillis urlToLocationDescription(delegateUri, programEnvironment).right.map(MockPathLocationDescription(delay, _)) } catch { - case _: ConfigException => Left(s"not a mock location: $url") + case _: ConfigException => Left("not a mock location") } } case "s3" => @@ -624,8 +624,8 @@ object LocationDescription extends StrictLogging { objectKey ) ) - case Some(l) if l.hasError => return Left(l.getError.getMessage) - case Some(_) => Left("missing S3 credential") + case Some(l) if l.hasError => Left(l.getError.getMessage) + case Some(_) => Left("not a S3 credential") case None => // Anonymous access. Right(S3PathLocationDescription(bucketName, None, None, None, objectKey)) @@ -669,7 +669,8 @@ object LocationDescription extends StrictLogging { } else { Left("missing Dropbox credential") } - case Some(l) if l.hasError => return Left(l.getError.getMessage) + case Some(l) if l.hasError => Left(l.getError.getMessage) + case Some(_) => Left("not a Dropbox credential") case None => Left("missing Dropbox credential") } case _ => Left(s"unsupported protocol: $protocol") From 504064fe31f77a4f87606c795ba23366ce5a32e4 Mon Sep 17 00:00:00 2001 From: Benjamin Gaidioz Date: Tue, 13 Aug 2024 15:39:09 +0200 Subject: [PATCH 6/6] Adding logging --- .../scala/raw/compiler/rql2/api/LocationDescription.scala | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/snapi-frontend/src/main/scala/raw/compiler/rql2/api/LocationDescription.scala b/snapi-frontend/src/main/scala/raw/compiler/rql2/api/LocationDescription.scala index 29aaa3d58..676c3d8cb 100644 --- a/snapi-frontend/src/main/scala/raw/compiler/rql2/api/LocationDescription.scala +++ b/snapi-frontend/src/main/scala/raw/compiler/rql2/api/LocationDescription.scala @@ -638,6 +638,7 @@ object LocationDescription extends StrictLogging { // In Dropbox, the host is the name of the credential val DROPBOX_REGEX(name, path) = url if (name == null) { + logger.warn("missing 'name' in Dropbox location") return Left("missing Dropbox credential") } programEnvironment.locationConfigs.get(name) match { @@ -667,11 +668,14 @@ object LocationDescription extends StrictLogging { Left("invalid Dropbox credential") } } else { + logger.warn("missing Dropbox 'Authorization'") Left("missing Dropbox credential") } case Some(l) if l.hasError => Left(l.getError.getMessage) case Some(_) => Left("not a Dropbox credential") - case None => Left("missing Dropbox credential") + case None => + logger.warn("missing Dropbox credential") + Left("missing Dropbox credential") } case _ => Left(s"unsupported protocol: $protocol") }