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

Unix socket for the gRPC server #1217

Merged
merged 15 commits into from
May 21, 2020
Merged

Unix socket for the gRPC server #1217

merged 15 commits into from
May 21, 2020

Conversation

fntlnz
Copy link
Contributor

@fntlnz fntlnz commented May 18, 2020

What type of PR is this?

/kind feature

Any specific area of the project related to this PR?

/area engine

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #858

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

new: unix socket for the gRPC server

fntlnz and others added 14 commits May 19, 2020 14:36
…ring

Signed-off-by: Lorenzo Fontana <lo@linux.com>
Signed-off-by: Lorenzo Fontana <lo@linux.com>
Signed-off-by: Lorenzo Fontana <lo@linux.com>
Signed-off-by: Lorenzo Fontana <lo@linux.com>
Signed-off-by: Lorenzo Fontana <lo@linux.com>
Signed-off-by: Lorenzo Fontana <lo@linux.com>
Signed-off-by: Lorenzo Fontana <lo@linux.com>
The CMake module downloads `string-view-lite` from
https://github.com/martinmoene/string-view-lite

It is a single-file header-only version of C++17-like `string_view` for
C++98, C++03, C++11, and later.

Notices it also provides C++20 extensions like:

- empty()
- starts_with()
- ends_with()
- etc.

Signed-off-by: Leonardo Di Donato <leodidonato@gmail.com>
Signed-off-by: Leonardo Di Donato <leodidonato@gmail.com>
…eader

Signed-off-by: Leonardo Di Donato <leodidonato@gmail.com>
…heme(string_view)

Also no more custom `starts_with` utility function.

Signed-off-by: Leonardo Di Donato <leodidonato@gmail.com>
Signed-off-by: Leonardo Di Donato <leodidonato@gmail.com>
Signed-off-by: Leonardo Di Donato <leodidonato@gmail.com>
Signed-off-by: Leonardo Di Donato <leodidonato@gmail.com>
Copy link
Member

@leodido leodido left a comment

Choose a reason for hiding this comment

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

🌮

@poiana
Copy link

poiana commented May 21, 2020

LGTM label has been added.

Git tree hash: 80f55cf073856fd86f44930b4b96c939f3dc7415

Copy link
Contributor

@mstemm mstemm left a comment

Choose a reason for hiding this comment

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

lgtm! Only suggestion is making a unix socket the default but I don't have a strong preference on it.

# By default the gRPC server is off.
# You can configure the address to bind and expose it.
# By modifying the threadiness configuration you can fine-tune the number of threads (and context) it will use.
# grpc:
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't have a strong preference, but maybe we should make a unix socket the default?

@poiana poiana merged commit f186e5f into master May 21, 2020
@poiana poiana deleted the feat/grpc-unix branch May 21, 2020 16:16
@poiana
Copy link

poiana commented May 21, 2020

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: leodido, mstemm

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

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

Successfully merging this pull request may close these issues.

GRPC Output: Also support unix domain sockets
5 participants