Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/bom #622

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Feature/bom #622

wants to merge 8 commits into from

Conversation

jawaff
Copy link

@jawaff jawaff commented Nov 20, 2019

I made a mistake trying to put BOM files into a separate repository. This pull request is what I should have done since the beginning. I thought that the Light platform was split into a bunch of different repositories, but there's only a few main multi-module projects and various libraries (which are kind of independent). Most of my Microservice experience is with Gradle and our application was split into a bunch of different repositories instead of having a few repositories filled with a bunch of modules.

Tonight I've started to have second thoughts about the way my team decided to structure our application. This experience has been enlightening.

The work I've done in this pull request is based on strategies used in the Spring and Jersey projects. Their project structures are very similar to the light-4j and light-rest-4j projects.

Check the bom/README.md file for documentation on the strategy I've taken.

…ugin for ensuring no dependency conflicts exist within this project.
…pendency management sections. Also fixed a few problems I introduced.
@jawaff
Copy link
Author

jawaff commented Nov 20, 2019

Hooray, the build failed and caught something potentially problematic!

By adding the maven-enforcer-plugin to the light-4j build we've found some dependency conflicts that were silently hiding within this project for potentially a long time. This can cause dependencies used for compilation to be potentially different than dependencies at runtime. That may or may not be a problem, but getting rid of the dependency conflicts ensures that there can't be a problem like that.

@jawaff
Copy link
Author

jawaff commented Nov 20, 2019

It turns out that the dependency conflicts were only in the test dependencies, but at least the maven-enforcer-plugin works as intended. I think I'm done adding a BOM to this project. I'll move onto the light-rest-4j project next.

@jawaff jawaff requested a review from stevehu November 20, 2019 14:03
@codecov-io
Copy link

Codecov Report

Merging #622 into master will decrease coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #622      +/-   ##
============================================
- Coverage     50.12%   50.11%   -0.02%     
+ Complexity     1922     1920       -2     
============================================
  Files           266      266              
  Lines         11582    11582              
  Branches       1648     1648              
============================================
- Hits           5806     5804       -2     
  Misses         5095     5095              
- Partials        681      683       +2
Impacted Files Coverage Δ Complexity Δ
...n/java/com/networknt/client/oauth/OauthHelper.java 36.03% <0%> (-0.61%) 25% <0%> (-2%)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e6ab7dd...8777c26. Read the comment docs.

@jawaff
Copy link
Author

jawaff commented Nov 20, 2019

Sorry if I'm merging into the wrong branch. I figured that I'd add the BOM to the latest version of the project.

@stevehu
Copy link
Contributor

stevehu commented Nov 20, 2019

The master is the right branch. There are still a lot of users working with the 1.6.x branch as they cannot move to Java 11 due to some internal security policy constraints. Potentially, we might need to merge into the 1.6.x branch with git cherry-pick.

@stevehu
Copy link
Contributor

stevehu commented Nov 24, 2019

In the last two days, I have been reading about the BOM between different tasks and trying to understand how it works. I think your idea is to have a BOM module for each repository. In the light-rest-4j, we have both swagger and openapi supported. For any user, only one of them is used. How do we create two BOMs?

Even in the light-4j, there are still some combinations for example, with consul registration or not. Or which metrics module is used between influxdb and Prometheus. What is the best way to create several BOMs per repository?

@stevehu
Copy link
Contributor

stevehu commented Nov 25, 2019

I tried to release a snapshot to the maven central and it failed with the following error message.

[ERROR] Failed to execute goal org.apache.maven.plugins:maven-deploy-plugin:2.7:deploy (default-deploy) on project light-4j-bom: Deployment failed: repository element was not specified in the POM inside distributionManagement element or in -DaltDeploymentRepository=id::layout::url parameter -> [Help 1]

From the error message, I guess the bom is a sub module of the light-4j parent pom but in the pom.xml of bom module, there is no parent specified.

If you look at this example the parent is specified.

If you want to reproduce the issue, please run mvn clean install deploy.

@jawaff
Copy link
Author

jawaff commented Dec 3, 2019

Sorry for the delay @stevehu. I'm back from my holiday travelings and transitioning into a new job. I hope you've been well.

Anyway, I was a bit confused about BOM files toward the beginning of this adventure. What I didn't realize was that there are basically two types of BOMs. One BOM type strictly defines 3rd party dependency versions and is good for deciding on compatible 3rd party versions for one to many projects. The other BOM type sets the compatible versions of in-house dependencies, which also have transitive dependencies (these could enforce compatible 3rd party versions).

There's a bit more to keep in mind. BOM files are only supposed to affect the dependencyManagement section a POM file. The imported BOM's dependencyManagement versions can also be overridden or excluded if necessary. The main point is that the dependencyManagement element only enforces versions, it doesn't mean the dependency must be used.

At my workplace we had a single BOM for all of our projects (similar to the light-bom project I was toying with). We did that so that our microservices could be loaded into a single JVM without the possibility of the wrong jar version being used at runtime.

To answer your light-rest-4j question, there's a few ways to go about this (I'm not sure what the best is). A BOM is essentially a parent pom file that only defines a dependencyManagement element so that it can be imported. It doesn't really hurt if extra things are added to the dependencyManagement, because they aren't necessarily used and can be excluded. So maybe a single BOM might be acceptable for this scenario. However, it's also possible to have a two BOMs that share a parent BOM. Transitive dependencies work well in BOMs. There can also be just two BOMs that define dependencies for both of the customer situations you were referring to (independently I mean).

@stevehu
Copy link
Contributor

stevehu commented Dec 3, 2019

@jawaff Congratulations on your new job. You are such a young energic developer and there would be more opportunities waiting for you.

Thanks for the clarification. Now I understand that the BOM doesn't mean the dependencies will be included into the final fat jar. In this case, I think one BOM per for light-rest-4j is a better solution as you have suggested.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants