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

[PR] Move Catalog ITs to embedded tests (druid)

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

   ### Changes
   
   - Move Catalog revised ITs to embedded tests
   - Refactor `CatalogClient` into `TestCatalogClient`
   - Remove code for backward compatibility IT as that is already handled by `IngestionDockerBackwardCompatibilityTest`
   
   <hr>
   
   This PR has:
   
   - [ ] been self-reviewed.
      - [ ] using the [concurrency checklist](https://github.com/apache/druid/blob/master/dev/code-review/concurrency.md) (Remove this item if the PR doesn't have any relation to concurrency.)
   - [ ] added documentation for new or modified features or behaviors.
   - [ ] a release note entry in the PR description.
   - [ ] added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
   - [ ] added or updated version, license, or notice information in [licenses.yaml](https://github.com/apache/druid/blob/master/dev/license.md)
   - [ ] added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
   - [ ] added unit tests or modified existing tests to cover new code paths, ensuring the threshold for [code coverage](https://github.com/apache/druid/blob/master/dev/code-review/code-coverage.md) is met.
   - [ ] added integration tests.
   - [ ] been tested in a test Druid cluster.
   


-- 
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] Move Catalog ITs to embedded tests (druid)

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


##########
embedded-tests/src/test/java/org/apache/druid/testing/embedded/auth/BasicAuthMSQTest.java:
##########
@@ -86,14 +80,6 @@ protected EmbeddedDruidCluster createCluster()
         .addServer(overlord)
         .addServer(indexer)
         .addServer(new EmbeddedBroker())
-        .addExtensions(

Review Comment:
   is this just cleanup needed after moving MSQ from extension to core druid module?



-- 
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] Move Catalog ITs to embedded tests (druid)

Posted by "kfaraz (via GitHub)" <gi...@apache.org>.
kfaraz commented on PR #18412:
URL: https://github.com/apache/druid/pull/18412#issuecomment-3245697201

   Thanks a lot for the review, @capistrant !


-- 
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] Move Catalog ITs to embedded tests (druid)

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


##########
embedded-tests/src/test/java/org/apache/druid/testing/embedded/auth/BasicAuthMSQTest.java:
##########
@@ -86,14 +80,6 @@ protected EmbeddedDruidCluster createCluster()
         .addServer(overlord)
         .addServer(indexer)
         .addServer(new EmbeddedBroker())
-        .addExtensions(

Review Comment:
   Yes, this had been missed in the original MSQ migration PR.



-- 
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] Move Catalog ITs to embedded tests (druid)

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


-- 
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] Move Catalog ITs to embedded tests (druid)

Posted by "kfaraz (via GitHub)" <gi...@apache.org>.
kfaraz commented on PR #18412:
URL: https://github.com/apache/druid/pull/18412#issuecomment-3201427528

   CI is failing due to `CompactionTaskTest` not being able to reserve direct memory.
   If several embedded clusters are started and stopped within the same JVM, the buffers that each service
   allocates for merging pools does not get freed, probably due to some resource leakage.
   
   Currently investigating this.


-- 
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