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

Remove lightblue-rest-integration-test as a dependency for lightblue-client #267

Open
jewzaam opened this issue Mar 2, 2016 · 16 comments
Open
Labels

Comments

@jewzaam
Copy link
Member

jewzaam commented Mar 2, 2016

This is leading to dependency hell for releases. Client shouldn't require either of these! We're looking at releasing only 4.0.0 now and it is dependent on changes in rest at least...

@jewzaam jewzaam added the bug label Mar 2, 2016
@paterczm
Copy link
Contributor

paterczm commented Mar 2, 2016

lightblue-rest is a dependency because it's used in integration testing (lightblue-client-integration-test). lightblue-client-integration-test used to be standalone, but at some point it was added to the client.

@paterczm
Copy link
Contributor

paterczm commented Mar 2, 2016

And lightblue-core-hystrix dependency is used because client uses hystrix. Since we're considering to drop hystrix altogether, we can start by removing it from the client.

@paterczm
Copy link
Contributor

paterczm commented Mar 2, 2016

@dcrissman, fyi.

@dcrissman
Copy link
Member

com.redhat.lightblue.rest:lightblue-rest-integration-test makes LightblueClientTestHarness possible

lightblue-client-integration-test was added to the lightblue-client because it makes sense that the client would include tools for testing using the client.

As for hystrix, I have no objections to it being removed if we are removing hystrix anyway.

@jewzaam
Copy link
Member Author

jewzaam commented Mar 2, 2016

Re lightblue-client-integration-test, I'd recommend breaking into a
separate repo so we don't tie client to rest. We can incorporate it in the
lightblue repo as a submodule and have it built every time that is built
(nightly internally and when touched via travis).

Hystrix we had talked about breaking out. Thought we were considering
keeping it for client only. If so, hystrix could move to client?

On Wed, Mar 2, 2016 at 5:30 PM, Dennis Crissman [email protected]
wrote:

com.redhat.lightblue.rest:lightblue-rest-integration-test makes
LightblueClientTestHarness possible

lightblue-client-integration-test was added to the lightblue-client
because it makes sense that the client would include tools for testing
using the client.

As for hystrix, I have no objections to it being removed if we are
removing hystrix anyway.


Reply to this email directly or view it on GitHub
#267 (comment)
.

@dcrissman
Copy link
Member

Re lightblue-client-integration-test, I'd recommend breaking into a separate repo so we don't tie client to rest. We can incorporate it in the lightblue repo as a submodule and have it built every time that is built (nightly internally and when touched via travis).

This change would also force us to move integration testing, or testing in the client that is based on the LightblueClientTestHarness, to this external project also.

What dependency hell were you running into? I assume you wanted to release an application using the new version of the client without releasing the rest layer, but I think that is somewhat problematic as well as it assumes there were no breaking changes in the rest layer.

@jewzaam
Copy link
Member Author

jewzaam commented Mar 16, 2016

Wanted to release the client without releasing core and rest. There isn't
a good reason in my mind to have these dependencies with a JSON based REST
interface. There's no code to share. Client doesn't need to even do
validation. If there's a change in rest then yes client needs rest to be
released, but that's not a build-time dependency.

On Wed, Mar 16, 2016 at 11:04 AM, Dennis Crissman [email protected]
wrote:

Re lightblue-client-integration-test, I'd recommend breaking into a
separate repo so we don't tie client to rest. We can incorporate it in the
lightblue repo as a submodule and have it built every time that is built
(nightly internally and when touched via travis).

This change would also force us to move integration testing, or testing in
the client that is based on the LightblueClientTestHarness, to this
external project also.

What dependency hell were you running into? I assume you wanted to release
an application using the new version of the client without releasing the
rest layer, but I think that is somewhat problematic as well as it assumes
there were no breaking changes in the rest layer.


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#267 (comment)

@dcrissman
Copy link
Member

In order to remove hystrix from lightblue-client, we would need to extract lightblue-core-hystrix into it's own library also. I am moving the hystrix element of this ticket to #273.

Another issue is that now we have removed jdk7 from core/mongo/rest and started using jdk8, the jdk7 build we had talked about continuing to support for the client is failing.

@dcrissman dcrissman changed the title Remove deps on lightblue core and rest!!!!! Remove lightblue-rest-integration-test as a dependency for lightblue-client Mar 17, 2016
@dcrissman
Copy link
Member

https://github.com/lightblue-platform/lightblue-test-utils was the former project name. I am thinking of re-using this repo, but I might rename it to lightblue-client-test. Objections? Suggestions?

@dcrissman
Copy link
Member

Splitting the module will not fully solve the jdk7/8 problem. Any application running jdk7 and using the client-test module will be locked in at the last version built as only jdk8 builds can be made moving forward as it will depend on lightblue-rest.

@jewzaam
Copy link
Member Author

jewzaam commented Mar 17, 2016

The lightblue-rest dependency needs to be broken out and support 1.7 and
1.8 then, right?

On Thu, Mar 17, 2016 at 3:57 PM, Dennis Crissman [email protected]
wrote:

Splitting the module will not fully solve the jdk7/8 problem. Any
application running jdk7 and using the client-test module will be locked in
at the last version built as only jdk8 builds can be made moving forward as
it will depend on lightblue-rest.


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#267 (comment)

@dcrissman
Copy link
Member

That is a path we probably don't want to go down. As lightblue-rest-integration-test depends on lightblue-mongo-integration-test which depends on lightblue-core-integration-test which depends on LightblueFactory which puts us back to supporting both 7 and 8.

@jewzaam
Copy link
Member Author

jewzaam commented Mar 17, 2016

Then how is this going to be done? Don't the tests in client depend on
those integration tests?

On Thu, Mar 17, 2016 at 4:46 PM, Dennis Crissman [email protected]
wrote:

That is a path we probably don't want to go down. As
lightblue-rest-integration-test depends on lightblue-mongo-integration-test
which depends on lightblue-core-integration-test which depends on
LightblueFactory which puts us back to supporting both 7 and 8.


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#267 (comment)

@dcrissman
Copy link
Member

Not all the tests in lightblue-client, just the integration tests that are located at https://github.com/lightblue-platform/lightblue-client/tree/master/lightblue-client-integration-test/src/test/java/com/redhat/lightblue/client/integration/test

Tests in lightblue-client-core/http/hystrix do not have this dependency.

It will be awkward to have integration tests that live elsewhere, but if we want lightblue-client to support both jdk versions, then I don't see a way around it.

@dcrissman
Copy link
Member

Discussion got cross-pollinated with #277

@jewzaam
Copy link
Member Author

jewzaam commented Mar 21, 2016

Core dependency removed with #277
Rest dependency remains.

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

No branches or pull requests

3 participants