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.

Sign up for GitHub

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

Jump to bottom

Mount storage service on zipkin-server #58

Merged
merged 10 commits into from
Oct 30, 2019
Merged

Mount storage service on zipkin-server #58

merged 10 commits into from
Oct 30, 2019

Conversation

jeqo
Copy link
Collaborator

@jeqo jeqo commented Oct 18, 2019

Fix #40

@jeqo jeqo requested a review from anuraaga October 18, 2019 13:01
@codefromthecrypt
Copy link
Contributor

missing files?

codefromthecrypt
codefromthecrypt reviewed Oct 18, 2019
View reviewed changes
storage/pom.xml Outdated Show resolved Hide resolved
anuraaga
anuraaga approved these changes Oct 21, 2019
View reviewed changes
Copy link
Contributor

@anuraaga anuraaga left a comment

Choose a reason for hiding this comment

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

LGTM on top of current comments :)

@jeqo
Copy link
Collaborator Author

jeqo commented Oct 23, 2019

@anuraaga any hint on what is causing consumer<serverBuilder> to not be properly mounted? I've debugged and method is been injected properly but endpoint is not available. With ArmeriaServerConfigurator works fine tho.

@anuraaga
Copy link
Contributor

Ugh looks like a bug

https://github.com/line/armeria/blob/master/spring/boot-autoconfigure/src/main/java/com/linecorp/armeria/spring/ArmeriaAutoConfiguration.java#L81

That should contain Consumer too. Unfortunately every type in there is part of armeria-spring so it won't help too try to work around it. Sorry to cause debug headache...

@anuraaga
Copy link
Contributor

line/armeria#2211 is the fix

This was referenced Oct 25, 2019
Merged
Merged
@jeqo jeqo reopened this Oct 29, 2019
@jeqo
Copy link
Collaborator Author

jeqo commented Oct 30, 2019

Thanks for the fix @anuraaga !

I will go and merge this with ArmeriaServerConfigurator until next Zipkin release is available :) #61

@jeqo jeqo merged commit adf3b8c into master Oct 30, 2019
@jeqo jeqo deleted the same-server branch October 30, 2019 09:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@codefromthecrypt codefromthecrypt codefromthecrypt left review comments

@anuraaga anuraaga anuraaga approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Reuse zipkin armeria server to expose kafka-streams scatter-gather API
3 participants
@jeqo @codefromthecrypt @anuraaga