[GitHub] [druid] gianm commented on pull request #10085: Filter http requests by http method

[GitBox <gi...@apache.org>]

gianm commented on pull request #10085:
URL: https://github.com/apache/druid/pull/10085#issuecomment-651275412


   > CI will fail because of missing code coverage for adding the filter to all the jetty services. The webconsole e2e test covers this, so would it be ok to overrule CI in this instance?
   
   IMO it's best to deal with it somehow in this PR, rather than kicking the can down the road. I've run into an issue like this before too, where the tests for some druid-processing classes live in the druid-sql module, and the checker can't realize it. In my case I just added extra tests to the druid-processing module.
   
   Some options that could work here:
   
   - Add extra tests to the druid-services module.
   - Add coverage suppressions for these files (https://github.com/apache/druid/blob/master/pom.xml#L1259) with a note like "these druid-services classes are tested by tests in the druid-XXX module(s)".
   - Modify the coverage checker to unify coverage across all modules (this is ideal in general, but might be too much work to ask for in this particular 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.

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