[ 
https://issues.apache.org/jira/browse/DRILL-8430?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17721253#comment-17721253
 ] 

ASF GitHub Bot commented on DRILL-8430:
---------------------------------------

jnturton commented on code in PR #2800:
URL: https://github.com/apache/drill/pull/2800#discussion_r1189557009


##########
exec/java-exec/src/main/java/org/apache/drill/exec/store/http/oauth/OAuthUtils.java:
##########
@@ -36,6 +37,7 @@
 
 public class OAuthUtils {
   private static final Logger logger = 
LoggerFactory.getLogger(OAuthUtils.class);
+  private static final ObjectMapper MAPPER = JacksonUtils.createObjectMapper();

Review Comment:
   We're converting method scope ObjectMappers to static class members which is 
efficient in terms of rework but will add a bit to Drill's fixed heap 
requirement since they can never be collected. It looks like ObjectMappers are 
thread safe so, for the cases where the caller does not need to do mapper 
customisation, could we get even better reuse from a new singleton 
`JacksonUtils.DEFAULT_MAPPER`?



##########
common/src/test/java/org/apache/drill/test/DrillTest.java:
##########
@@ -37,11 +38,10 @@
 
 public class DrillTest extends BaseTest {
 
-  protected static final ObjectMapper objectMapper;
+  private static final ObjectMapper objectMapper = 
JacksonUtils.createObjectMapper();

Review Comment:
   Nice catch, thank you.





> add factory method for creating Jackson ObjectMappers
> -----------------------------------------------------
>
>                 Key: DRILL-8430
>                 URL: https://issues.apache.org/jira/browse/DRILL-8430
>             Project: Apache Drill
>          Issue Type: Task
>          Components:  Server
>            Reporter: PJ Fanning
>            Priority: Major
>
> See https://issues.apache.org/jira/browse/DRILL-8415
> It's useful to keep any customisation of the ObjectMapper creation in 1 placeĀ 



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to