-
Notifications
You must be signed in to change notification settings - Fork 52
Allow a default source path to be specified via envvar #397
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Adam Wolfe Gordon <[email protected]>
Signed-off-by: Adam Wolfe Gordon <[email protected]>
fn_test.go
Outdated
@@ -952,6 +973,9 @@ func TestRunFunction(t *testing.T) { | |||
|
|||
for name, tc := range cases { | |||
t.Run(name, func(t *testing.T) { | |||
// NOTE: This means we can't run tests in parallel. | |||
defaultSource = tc.args.defaultSource |
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.
is there an alternative approach you were considering that doesn't rely on global variables?
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.
Not sure why I didn't do this initially, but we can read the envvar when we construct the Function
and store it in a field. This is tidier than the global variable for sure. Updated to do things that way.
Read the default source envvar as part of initialization in `main` and store it in the `Function` struct rather than using a global variable. This is tidier and lets us run our tests in parallel. Signed-off-by: Adam Wolfe Gordon <[email protected]>
fsys: &osFS{}, | ||
log: log, | ||
fsys: &osFS{}, | ||
defaultSource: os.Getenv("FUNCTION_GO_TEMPLATING_DEFAULT_SOURCE"), |
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.
awesome, i like this approach of resolving the value in main.go and passing it into the function more than a global var, thanks for that consideration!
now i'm also wondering if this should be exposed as a command line arg, in addition to the env var, similar to TLSCertsDir
. i'm not sure if it should be exposed at that level also, but it's something else to consider here 🤔
Description of your changes
Enable function-go-templating to be used as a base image for custom functions with go templates included in the image by allowing the function input to be empty when a default source is set via environment variable. A user can build a custom function on top of function-go-templating by adding a directory containing go templates to the image and setting the environment variable
FUNCTION_GO_TEMPLATING_DEFAULT_SOURCE
to the template directory's path. The behavior of the function will be the same as if the templates were mounted into the function pod and theFilesystem
input source used.I have: