> On Nov. 22, 2023, 10:27 a.m., Vikas Kumar wrote:
> > security-admin/pom.xml
> > Lines 320 (patched)
> > <https://reviews.apache.org/r/74228/diff/1/?file=2272240#file2272240line320>
> >
> >     Why do we need to define this dependency explicitly ?
> >     
> >     KMS (may be some other modules as well) has transitive dependency on 
> > "okhttp" and "okhttp-urlconnection". I can't find any direct dependency on 
> > these.
> >     
> >     So, simply updating the version of okhttp in distro/kms.xml should be 
> > sufficient. 
> >     
> >     Please let me know your thoughts on this.
> 
> bhavik patel wrote:
>     There is no direct dependency, but to avoid transitive dependency I have 
> included Separately.
> 
> Vikas Kumar wrote:
>     Sorry, I am not getting this. This way we will have to define exact 
> version for all transitive dependencies, and that too in all modules. 
>     Instead of that, can't we simply update in distro that defines the 
> runtime required libs. 
>     
>     Can you help me with the Admin lib jar that has indirect dependency on 
> okhttp?
> 
> bhavik patel wrote:
>     Directly defining in distro will only help when we get that exact version 
> from the trasitive dependency. 
>     
>     I will revisite my changes and update the patch if required in next week.

Hi Bhavik, I tried my approach to directly updating the version in distro file 
but it is getting more complicated, instead let application moduless define 
their dependencies. And their your approach makes more sense to me now.

But shouldn't we consider following:

Instead of defining dependencies in module's pom, use <dependencyManagement> to 
override in parent pom and exclude from respective module's pom.

For ex: for your case, exclude from admin pon and override using 
<dependencyManagement> in parent pom?

What's your thought on this?

PS: Dropping my earlier review comment.


- Vikas


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/74228/#review225989
-----------------------------------------------------------


On Nov. 22, 2023, 4:27 a.m., bhavik patel wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/74228/
> -----------------------------------------------------------
> 
> (Updated Nov. 22, 2023, 4:27 a.m.)
> 
> 
> Review request for ranger, Dhaval Shah, Madhan Neethiraj, Pradeep Agrawal, 
> Ramesh Mani, and Vikas Kumar.
> 
> 
> Bugs: RANGER-3993
>     https://issues.apache.org/jira/browse/RANGER-3993
> 
> 
> Repository: ranger
> 
> 
> Description
> -------
> 
> Upgrade okhttp dependency
> 
> 
> Diffs
> -----
> 
>   pom.xml 7b0dd14c5 
>   security-admin/pom.xml 5e24dd846 
> 
> 
> Diff: https://reviews.apache.org/r/74228/diff/1/
> 
> 
> Testing
> -------
> 
> Validated Ranger admin and KMS CRUD operations.
> 
> 
> Thanks,
> 
> bhavik patel
> 
>

Reply via email to