-
Notifications
You must be signed in to change notification settings - Fork 96
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
Tristan instacart/add basic ssl support #73
base: master
Are you sure you want to change the base?
Changes from all commits
4578102
4c9e5c8
ecbe239
f9672a5
b4a6006
3ff453a
4672cc3
1ebf0b7
f984a3b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,7 +15,11 @@ def self.generate | |
thread_id = Thread.current.object_id | ||
identity = "#{thread_id}@#{hostname}" | ||
|
||
client_class.new(host, port, identity) | ||
if Temporal.configuration.client_type == :grpc | ||
client_class.new(host, port, identity, Temporal.configuration.grpc_ssl_config) | ||
else | ||
client_class.new(host, port, identity) | ||
end | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you please add a test case for this? |
||
end | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,9 +22,10 @@ class GRPCClient | |
close: Temporal::Api::Enums::V1::HistoryEventFilterType::HISTORY_EVENT_FILTER_TYPE_CLOSE_EVENT, | ||
}.freeze | ||
|
||
def initialize(host, port, identity) | ||
def initialize(host, port, identity, grpc_ssl_config) | ||
@url = "#{host}:#{port}" | ||
@identity = identity | ||
@channel_creds = grpc_ssl_config | ||
@poll = true | ||
@poll_mutex = Mutex.new | ||
@poll_request = nil | ||
|
@@ -388,12 +389,12 @@ def cancel_polling_request | |
|
||
private | ||
|
||
attr_reader :url, :identity, :poll_mutex, :poll_request | ||
attr_reader :url, :channel_creds, :identity, :poll_mutex, :poll_request | ||
|
||
def client | ||
@client ||= Temporal::Api::WorkflowService::V1::WorkflowService::Stub.new( | ||
url, | ||
:this_channel_is_insecure, | ||
channel_creds, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And this? |
||
timeout: 60 | ||
) | ||
end | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,7 +4,7 @@ | |
module Temporal | ||
class Configuration | ||
attr_reader :timeouts, :error_handlers | ||
attr_accessor :client_type, :host, :port, :logger, :metrics_adapter, :namespace, :task_queue, :headers | ||
attr_accessor :channel_creds, :client_type, :host, :port, :logger, :metrics_adapter, :namespace, :task_queue, :headers | ||
|
||
# We want an infinite execution timeout for cron schedules and other perpetual workflows. | ||
# We choose an 10-year execution timeout because that's the maximum the cassandra DB supports, | ||
|
@@ -32,6 +32,7 @@ def initialize | |
@namespace = DEFAULT_NAMESPACE | ||
@task_queue = DEFAULT_TASK_QUEUE | ||
@headers = DEFAULT_HEADERS | ||
@channel_creds = nil | ||
@error_handlers = [] | ||
end | ||
|
||
|
@@ -50,5 +51,13 @@ def task_list=(name) | |
def timeouts=(new_timeouts) | ||
@timeouts = DEFAULT_TIMEOUTS.merge(new_timeouts) | ||
end | ||
|
||
def grpc_ssl_config | ||
if @channel_creds.nil? | ||
:this_channel_is_insecure | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. also would be great to have a spec for this and also put the value in a constant |
||
else | ||
@channel_creds | ||
end | ||
end | ||
end | ||
end |
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.
nit: Ruby folks are ok with full names, let's probably call this just
credentials
unless you think it might conflict with some other config we'd want to add in the future