-
Notifications
You must be signed in to change notification settings - Fork 23
Add support to reload environment variables #20
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
Conversation
@pragnagopa - could you add some context for this PR in the description? |
@@ -180,6 +184,16 @@ message WorkerStatusRequest{ | |||
message WorkerStatusResponse { | |||
} | |||
|
|||
message FunctionEnvironmentLoadRequest { | |||
// Environment variables from the current process | |||
map<string, string> environment_variables = 1; |
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.
Can an environment variable's value ever be null or empty? languages like javascript will buffer overflow if so :|
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.
we can control this in host.
src/proto/FunctionRpc.proto
Outdated
@@ -65,6 +65,10 @@ message StreamingMessage { | |||
|
|||
// Worker logs a message back to the host | |||
RpcLog rpc_log = 2; | |||
|
|||
FunctionEnvironmentLoadRequest function_environment_load_request = 25; |
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.
Can we call it something like "FunctionEnvironmentReloadRequest", since environment variables will be there by default for most cases?
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.
makes sense. will update
src/proto/FunctionRpc.proto
Outdated
@@ -180,6 +184,16 @@ message WorkerStatusRequest{ | |||
message WorkerStatusResponse { | |||
} | |||
|
|||
message FunctionEnvironmentLoadRequest { |
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.
I think you might have missed renaming the message names!
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.
duh..yes. thanks!
Added new message to support reloading of environment variables. This message is specifically used during specialization when functions runtime starts a language worker in place holder mode and then assigns to an app at a later point in time.