sungwy commented on code in PR #3042:
URL: https://github.com/apache/polaris/pull/3042#discussion_r2529938499
##########
extensions/auth/opa/impl/src/main/java/org/apache/polaris/extension/auth/opa/OpaPolarisAuthorizer.java:
##########
@@ -174,27 +177,35 @@ private boolean queryOpa(
httpPost.setEntity(new StringEntity(inputJson,
ContentType.APPLICATION_JSON));
// Execute request
- try (CloseableHttpResponse response = httpClient.execute(httpPost)) {
- int statusCode = response.getCode();
- if (statusCode != 200) {
- return false;
- }
-
- // Read and parse response
- String responseBody;
- try {
- responseBody = EntityUtils.toString(response.getEntity());
- } catch (ParseException e) {
- throw new RuntimeException("Failed to parse OPA response", e);
- }
- ObjectNode respNode = (ObjectNode) objectMapper.readTree(responseBody);
- return respNode.path("result").path("allow").asBoolean(false);
- }
- } catch (IOException e) {
+ return httpClientExecute(httpPost, this::queryOpaCheckResponse);
+ } catch (HttpException | IOException e) {
throw new RuntimeException("OPA query failed", e);
}
}
+ <T> T httpClientExecute(
Review Comment:
```suggestion
@VisibleForTesting
<T> T httpClientExecute(
```
It looks like we are following the convention of marking it as being visible
for testing, if that's our intent
https://github.com/apache/polaris/blob/286e77d5e3b6c0a9de051928471d0f9e3ec2d453/runtime/service/src/main/java/org/apache/polaris/service/storage/aws/StsClientsPool.java#L59-L70
What are your thoughts on adding it here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
