You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by "gianm (via GitHub)" <gi...@apache.org> on 2025/08/20 22:43:14 UTC

[PR] Consolidate ObjectMappers in MSQTestBase. (druid)

gianm opened a new pull request, #18422:
URL: https://github.com/apache/druid/pull/18422

   This patch consolidates the two ObjectMappers in MSQTestBase into a single ObjectMapper. The single ObjectMapper is based on the modules from the test class's ComponentSupplier. In addition to simplification, this has the benefit of making it easier to create subclasses that provide additional Jackson modules.


-- 
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: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


Re: [PR] Consolidate ObjectMappers in MSQTestBase. (druid)

Posted by "gianm (via GitHub)" <gi...@apache.org>.
gianm merged PR #18422:
URL: https://github.com/apache/druid/pull/18422


-- 
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: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


Re: [PR] Consolidate ObjectMappers in MSQTestBase. (druid)

Posted by "gianm (via GitHub)" <gi...@apache.org>.
gianm commented on code in PR #18422:
URL: https://github.com/apache/druid/pull/18422#discussion_r2292078003


##########
extensions-core/multi-stage-query/src/test/java/org/apache/druid/msq/test/MSQTestBase.java:
##########
@@ -429,13 +456,10 @@ public void setUp2() throws Exception
 
     SqlTestFramework qf = queryFramework();
 
-    ObjectMapper secondMapper = setupObjectMapper(qf.injector());
-    indexIO = new IndexIO(secondMapper, ColumnConfig.DEFAULT);
-
-    segmentCacheManager = new SegmentCacheManagerFactory(TestIndex.INDEX_IO, secondMapper).manufacturate(newTempFolder("cacheManager"));
-
-    MSQSqlModule sqlModule = new MSQSqlModule();
+    objectMapper = setupObjectMapper(qf.injector());
+    indexIO = new IndexIO(objectMapper, ColumnConfig.DEFAULT);
 
+    segmentCacheManager = new SegmentCacheManagerFactory(TestIndex.INDEX_IO, objectMapper).manufacturate(newTempFolder("cacheManager"));

Review Comment:
   Yes, just pushed that.



-- 
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: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


Re: [PR] Consolidate ObjectMappers in MSQTestBase. (druid)

Posted by "kfaraz (via GitHub)" <gi...@apache.org>.
kfaraz commented on code in PR #18422:
URL: https://github.com/apache/druid/pull/18422#discussion_r2289721399


##########
extensions-core/multi-stage-query/src/test/java/org/apache/druid/msq/test/MSQTestBase.java:
##########
@@ -429,13 +456,10 @@ public void setUp2() throws Exception
 
     SqlTestFramework qf = queryFramework();
 
-    ObjectMapper secondMapper = setupObjectMapper(qf.injector());
-    indexIO = new IndexIO(secondMapper, ColumnConfig.DEFAULT);
-
-    segmentCacheManager = new SegmentCacheManagerFactory(TestIndex.INDEX_IO, secondMapper).manufacturate(newTempFolder("cacheManager"));
-
-    MSQSqlModule sqlModule = new MSQSqlModule();
+    objectMapper = setupObjectMapper(qf.injector());
+    indexIO = new IndexIO(objectMapper, ColumnConfig.DEFAULT);
 
+    segmentCacheManager = new SegmentCacheManagerFactory(TestIndex.INDEX_IO, objectMapper).manufacturate(newTempFolder("cacheManager"));

Review Comment:
   Nit: Could we use the above `indexIO` here instead of `TestIndex.INDEX_IO`?



-- 
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: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org