-
Notifications
You must be signed in to change notification settings - Fork 35
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
Run embedded metadata store on node grpc server #2185
Run embedded metadata store on node grpc server #2185
Conversation
this will partially resolve #1609 ? |
e8fb789
to
15dfa5a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @tillrohrmann for creating this PR. It looks good to me (and makes sense) the only part I have questions over is the Replace QueryStorage grpc with QueryContext
commit! I don't see how this relate to embedding medatadata store on node grpc service
and feels out of place (as far as I can see) I must be missing some context regarding to this single commit.
So my question is why this commit is needed and how is it related to the rest of the PR.
I needed e40eee1 which needs |
15dfa5a
to
67a7bf2
Compare
Thanks for your review @muhamadazmy. I've rebased the PR onto the latest main. Hopefully, I could answer your question regarding the previous commits. |
Thanks for the reviews @muhamadazmy and @slinkydeveloper. Merging this PR now. |
By letting the embedded metadata store run on the node grpc server we can get rid of an additional address that users need to configure. Instead of being reachable under port 5123, the metadata store is now reachable under port 5122 by default.
67a7bf2
to
c1f742a
Compare
By letting the embedded metadata store run on the node grpc server
we can get rid of an additional address that users need to configure.
Instead of being reachable under port 5123, the metadata store is now
reachable under port 5122 by default.
This PR is based on #2182.
As a follow-up we need to update our materials to no longer mention the metadata store port.