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

fix issue#446 #447

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

fix issue#446 #447

wants to merge 5 commits into from

Conversation

7ue5wu
Copy link

@7ue5wu 7ue5wu commented May 22, 2023

Context

When running the source job example by following the Mantis document, error happened.

Exception in thread "main" java.lang.NoClassDefFoundError: com/netflix/spectator/api/Registry
Caused by: java.lang.ClassNotFoundException: com.netflix.spectator.api.Registr

Checklist

  • ./gradlew build compiles code correctly
  • Added new tests where applicable
  • ./gradlew test passes all tests
  • Extended README or added javadocs where applicable

Copy link
Collaborator

@Andyz26 Andyz26 left a comment

Choose a reason for hiding this comment

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

@7ue5wu 7ue5wu had a problem deploying to Integrate Pull Request May 25, 2023 02:23 — with GitHub Actions Failure
@7ue5wu
Copy link
Author

7ue5wu commented May 25, 2023

@7ue5wu you might want to update https://github.com/Netflix/mantis/blob/master/mantis-runtime/build.gradle too.

Hi Andy, I already updated this one too. Is it the right update? I am confused because it seems nothing different when running after I update this one.

@Andyz26
Copy link
Collaborator

Andyz26 commented May 25, 2023

@7ue5wu you might want to update https://github.com/Netflix/mantis/blob/master/mantis-runtime/build.gradle too.

Hi Andy, I already updated this one too. Is it the right update? I am confused because it seems nothing different when running after I update this one.

A bit of context: these were made compileOnly to fix some dynamic load problem in our internal wrapper layer and this is no longer needed now. Also, I don't know if you still need to declare the dep in the runtime module anymore if it's using "implementation" directly in common (in that case you can remove the part in runtime).

@7ue5wu 7ue5wu had a problem deploying to Integrate Pull Request May 26, 2023 04:03 — with GitHub Actions Failure
@7ue5wu
Copy link
Author

7ue5wu commented May 26, 2023

@7ue5wu you might want to update https://github.com/Netflix/mantis/blob/master/mantis-runtime/build.gradle too.

Hi Andy, I already updated this one too. Is it the right update? I am confused because it seems nothing different when running after I update this one.

A bit of context: these were made compileOnly to fix some dynamic load problem in our internal wrapper layer and this is no longer needed now. Also, I don't know if you still need to declare the dep in the runtime module anymore if it's using "implementation" directly in common (in that case you can remove the part in runtime).

It seems we should still keep this part temporarily because the ServerSentRequestHandler still needs the libraries for now. So I didn't change mantis-runtime/build.gradle and remain it onlyCompile. And the synthetic-sourcejob can run successfully.

@Andyz26
Copy link
Collaborator

Andyz26 commented May 26, 2023

@7ue5wu you might want to update https://github.com/Netflix/mantis/blob/master/mantis-runtime/build.gradle too.

Hi Andy, I already updated this one too. Is it the right update? I am confused because it seems nothing different when running after I update this one.

A bit of context: these were made compileOnly to fix some dynamic load problem in our internal wrapper layer and this is no longer needed now. Also, I don't know if you still need to declare the dep in the runtime module anymore if it's using "implementation" directly in common (in that case you can remove the part in runtime).

It seems we should still keep this part temporarily because the ServerSentRequestHandler still needs the libraries for now. So I didn't change mantis-runtime/build.gradle and remain it onlyCompile. And the synthetic-sourcejob can run successfully.

Since the common will now bring in the lib file, using "compileOnly" in the runtime will not remove it from the dep chain. I just feel that it's now a bit confusing using "compileOnly" in runtime when the lib is still going to be included.

@7ue5wu 7ue5wu requested a deployment to Integrate Pull Request May 27, 2023 08:13 — with GitHub Actions Waiting
@7ue5wu 7ue5wu had a problem deploying to Integrate Pull Request May 28, 2023 12:21 — with GitHub Actions Failure
@7ue5wu
Copy link
Author

7ue5wu commented May 28, 2023

@7ue5wu you might want to update https://github.com/Netflix/mantis/blob/master/mantis-runtime/build.gradle too.

Hi Andy, I already updated this one too. Is it the right update? I am confused because it seems nothing different when running after I update this one.

A bit of context: these were made compileOnly to fix some dynamic load problem in our internal wrapper layer and this is no longer needed now. Also, I don't know if you still need to declare the dep in the runtime module anymore if it's using "implementation" directly in common (in that case you can remove the part in runtime).

It seems we should still keep this part temporarily because the ServerSentRequestHandler still needs the libraries for now. So I didn't change mantis-runtime/build.gradle and remain it onlyCompile. And the synthetic-sourcejob can run successfully.

Since the common will now bring in the lib file, using "compileOnly" in the runtime will not remove it from the dep chain. I just feel that it's now a bit confusing using "compileOnly" in runtime when the lib is still going to be included.

Hi! I have already changed both of them to "Implementation", I think that can make sense this time and the example can run successfully now.

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.

3 participants