Skip to content

Commit

Permalink
DTS-3397: do not log URLs on access errors (#498)
Browse files Browse the repository at this point in the history
* DTS-3397: do not log URLs on access errors

* fix: use jdk 8 compatible version of mockito
  • Loading branch information
PatrickJin-db authored Jun 14, 2024
1 parent f0ae563 commit df8895f
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 3 deletions.
3 changes: 2 additions & 1 deletion build.sbt
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,8 @@ lazy val client = (project in file("client")) settings(
"org.apache.spark" %% "spark-catalyst" % sparkVersion % "test" classifier "tests",
"org.apache.spark" %% "spark-core" % sparkVersion % "test" classifier "tests",
"org.apache.spark" %% "spark-sql" % sparkVersion % "test" classifier "tests",
"org.scalatest" %% "scalatest" % "3.2.3" % "test"
"org.scalatest" %% "scalatest" % "3.2.3" % "test",
"org.scalatestplus" %% "mockito-4-11" % "3.2.18.0" % "test"
),
Compile / sourceGenerators += Def.task {
val file = (Compile / sourceManaged).value / "io" / "delta" / "sharing" / "client" / "package.scala"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ private[sharing] class RandomAccessHttpInputStream(
} else {
logDebug(s"Opening file $uri at pos $pos")

val entity = RetryUtils.runWithExponentialBackoff(numRetries, maxRetryDuration) {
val entity = RetryUtils.runWithExponentialBackoff(numRetries, maxRetryDuration) {
val httpRequest = createHttpRequest(pos)
val response = client.execute(httpRequest)
val status = response.getStatusLine()
Expand All @@ -167,7 +167,8 @@ private[sharing] class RandomAccessHttpInputStream(
}
}
throw new UnexpectedHttpStatus(
s"HTTP request failed with status: $status $errorBody, while accessing [$uri]",
s"HTTP request failed with status: $status $errorBody," +
s" while accessing URI of shared table file",
statusCode)
}
entity
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
/*
* Copyright (2021) The Delta Lake Project Authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package io.delta.sharing.client

import org.apache.hadoop.fs.FileSystem
import org.apache.http.{HttpStatus, ProtocolVersion}
import org.apache.http.client.HttpClient
import org.apache.http.message.BasicHttpResponse
import org.apache.spark.SparkFunSuite
import org.apache.spark.delta.sharing.PreSignedUrlFetcher
import org.mockito.ArgumentMatchers.any
import org.mockito.Mockito.when
import org.scalatestplus.mockito.MockitoSugar

import io.delta.sharing.client.util.UnexpectedHttpStatus

class RandomAccessHttpInputStreamSuite extends SparkFunSuite with MockitoSugar {

private def createResponse(status: Int): BasicHttpResponse = {
new BasicHttpResponse(new ProtocolVersion("HTTP", 1, 1), status, "")
}

private def createMockClient(status: Int): HttpClient = {
val client = mock[HttpClient]
when(client.execute(any())).thenReturn(createResponse(status))
client
}

private def createMockFetcher(uri: String): PreSignedUrlFetcher = {
val fetcher = mock[PreSignedUrlFetcher]
when(fetcher.getUrl()).thenReturn(uri)
fetcher
}

test("Failed HTTP requests should not show URI") {
val uri = "test.uri"
val stream = new RandomAccessHttpInputStream(
createMockClient(HttpStatus.SC_OK),
createMockFetcher(uri),
1000L,
new FileSystem.Statistics("idbfs"),
10
)
val error = intercept[UnexpectedHttpStatus] {
stream.seek(100L)
}
assert(!error.getMessage().contains(uri))
}
}

0 comments on commit df8895f

Please sign in to comment.