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

SAMZA-1677 : Make httpcore and httpclient dependencies consistent #534

Closed
wants to merge 1 commit into from

Conversation

cameronlee314
Copy link
Contributor

Samza currently depends on httpclient 4.5.2 and httpcore 4.4.5. However, httpclient 4.5.2 also has a direct dependency on httpcore 4.4.4, which is not backwards compatible with httpcore 4.4.5 since some classes were removed (e.g. ThreadSafe/NotThreadSafe annotation classes).

Although this does not currently cause any direct build problems, there may be cases where this conflict introduces transitive dependency conflicts. In addition, this inconsistency can cause confusion in future development if those libraries need to be used.

Copy link
Contributor

@vjagadish1989 vjagadish1989 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good to me. thanks Cameron for the contribution!

@@ -236,7 +236,6 @@ project(':samza-aws') {
compile project(":samza-core_$scalaVersion")
compile "org.slf4j:slf4j-api:$slf4jVersion"
runtime "org.apache.httpcomponents:httpclient:4.5.2"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Modules samza-yarn, samza-autoscaling are using httpClient version 4.4.1 through httpClientVersion variable defined in dependency-versions.gradle.

  • I think it will be better to use one consistent httpClient version across all samza modules.
  • Just wondering if samza-aws(using http-client:4.5.x) and samza-yarn(using http-client: 4.4.x) are used in same project, then would we run into same problem mentioned in the PR.

Either we can update other modules to use 4.5.x or downgrade this one to 4.4.x. Please change if this makes sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My observation was that the version backwards incompatibility problem is on httpcore (between 4.4.4 and 4.4.5), not httpclient. The httpclient:4.5.2 and httpclient:4.4.1 both depend on versions of httpcore which are 4.4.4 or lower, so there doesn't seem to be a backwards incompatibility problem there.
I do agree that it would be nice to have consistent versions, but I feel that is out of scope here and can be done in a separate PR which pulls in new versions for a bunch of other dependencies as well.

@prateekm
Copy link
Contributor

@cameronlee314 @shanthoosh What was the conclusion? Should this be merged?

@cameronlee314
Copy link
Contributor Author

This can be merged. Sorry for not following up on it earlier.

@asfgit asfgit closed this in c98d7b0 Jul 25, 2018
@cameronlee314 cameronlee314 deleted the httpcore branch October 4, 2019 21:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants