-
Notifications
You must be signed in to change notification settings - Fork 3
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
Add env vars for Workspace, Region and Branch #54
Changes from 5 commits
91cb8c2
aff8c86
e324ad2
418e8c5
13b2685
52291e0
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 |
---|---|---|
|
@@ -161,11 +161,11 @@ func loadConfig(fieName string) (config, error) { | |
return cfg, nil | ||
} | ||
|
||
// getBranchName retrieves the branch name. | ||
// If not found, falls back to defaultBranchName | ||
func getBranchName() string { | ||
if branchName, found := os.LookupEnv(branchNameEnvVar); found { | ||
return branchName | ||
// Get value from env var with fallback to godotenv | ||
// return default value if not found | ||
func getEnvVar(name string, defaultValue string) string { | ||
if val, found := os.LookupEnv(name); found { | ||
return val | ||
} | ||
|
||
var myEnv map[string]string | ||
|
@@ -177,11 +177,22 @@ func getBranchName() string { | |
} | ||
} | ||
|
||
if branchName, found := myEnv[branchNameEnvVar]; found { | ||
return branchName | ||
if val, found := myEnv[name]; found { | ||
return val | ||
} | ||
return defaultValue | ||
} | ||
|
||
return defaultBranchName | ||
// getBranchName retrieves the branch name. | ||
// If not found, falls back to defaultBranchName | ||
func getBranchName() string { | ||
return getEnvVar(branchNameEnvVar, defaultBranchName) | ||
} | ||
|
||
// Get the region if the corresponding env var `XATA_REGION` is set | ||
// otherwise return the default region: us-east-1 | ||
func getRegion() string { | ||
return getEnvVar("XATA_REGION", defaultRegion) | ||
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. nit: We appear to sometimes uses the raw string value of the environment variable names and sometimes constants. I think we should switch to constants everywhere. I'm happy to do this as a separate PR as doing it as part of this one could get messy. 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. that's a good nit, I felt guilty not doing it :) I'll push it up in a sec and share the commit. 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. 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. We still use the string values in a lot of tests. To get around this we'd have to make the constants public. |
||
} | ||
|
||
// loadDatabaseConfig will return config with defaults if the error is not nil. | ||
|
@@ -191,6 +202,21 @@ func loadDatabaseConfig() (databaseConfig, error) { | |
branchName: defaultBranchName, | ||
domainWorkspace: defaultDataPlaneDomain, | ||
} | ||
|
||
// Setup with env var | ||
// XATA_WORKSPACE_ID to set the workspace Id | ||
wsID := getEnvVar("XATA_WORKSPACE_ID", "") | ||
if wsID != "" { | ||
region := getRegion() | ||
branch := getBranchName() | ||
db := databaseConfig{ | ||
workspaceID: wsID, | ||
region: region, | ||
branchName: branch, | ||
} | ||
return db, nil | ||
} | ||
|
||
cfg, err := loadConfig(configFileName) | ||
if err != nil { | ||
return defaultDBConfig, err | ||
|
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 noticed that if the .env file doesn't exist we return an empty string instead of the default value, is that correct?
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.
Yes, that is correct. It's fine for now, as we only fetch an env var to bootstrap the connection params and if an empty string is encountered, continues to probe the
.xatarc
file.