-
-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Fix #1124 #1134
Merged
Merged
Fix #1124 #1134
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,7 +3,10 @@ package config | |
import ( | ||
"fmt" | ||
"path/filepath" | ||
"regexp" | ||
"strings" | ||
|
||
"github.com/hashicorp/go-getter" | ||
"github.com/hashicorp/hcl/v2" | ||
tflang "github.com/hashicorp/terraform/lang" | ||
"github.com/zclconf/go-cty/cty" | ||
|
@@ -404,6 +407,69 @@ func getCleanedTargetConfigPath(configPath string, workingPath string) string { | |
return util.CleanPath(targetConfig) | ||
} | ||
|
||
// If one of the xxx-all commands is called with the --terragrunt-source parameter, then for each module, we need to | ||
// build its own --terragrunt-source parameter by doing the following: | ||
// | ||
// 1. Read the source URL from the Terragrunt configuration of each module | ||
// 2. Extract the path from that URL (the part after a double-slash) | ||
// 3. Append the path to the --terragrunt-source parameter | ||
// | ||
// Example: | ||
// | ||
// --terragrunt-source: /source/infrastructure-modules | ||
// source param in module's terragrunt.hcl: git::[email protected]:acme/infrastructure-modules.git//networking/vpc?ref=v0.0.1 | ||
// | ||
// This method will return: /source/infrastructure-modules//networking/vpc | ||
// | ||
func GetTerragruntSourceForModule(sourcePath string, modulePath string, moduleTerragruntConfig *TerragruntConfig) (string, error) { | ||
if sourcePath == "" || moduleTerragruntConfig.Terraform == nil || moduleTerragruntConfig.Terraform.Source == nil || *moduleTerragruntConfig.Terraform.Source == "" { | ||
return "", nil | ||
} | ||
|
||
// use go-getter to split the module source string into a valid URL and subdirectory (if // is present) | ||
moduleUrl, moduleSubdir := getter.SourceDirSubdir(*moduleTerragruntConfig.Terraform.Source) | ||
|
||
// if both URL and subdir are missing, something went terribly wrong | ||
if moduleUrl == "" && moduleSubdir == "" { | ||
return "", errors.WithStackTrace(InvalidSourceUrl{ModulePath: modulePath, ModuleSourceUrl: *moduleTerragruntConfig.Terraform.Source, TerragruntSource: sourcePath}) | ||
} | ||
// if only subdir is missing, check if we can obtain a valid module name from the URL portion | ||
if moduleUrl != "" && moduleSubdir == "" { | ||
moduleSubdirFromUrl, err := getModulePathFromSourceUrl(moduleUrl) | ||
if err != nil { | ||
return moduleSubdirFromUrl, err | ||
} | ||
return util.JoinTerraformModulePath(sourcePath, moduleSubdirFromUrl), nil | ||
} | ||
|
||
return util.JoinTerraformModulePath(sourcePath, moduleSubdir), nil | ||
} | ||
|
||
// Parse sourceUrl not containing '//', and attempt to obtain a module path. | ||
// Example: | ||
// | ||
// sourceUrl = "git::ssh://[email protected]/OurOrg/module-name.git" | ||
// will return "module-name". | ||
|
||
func getModulePathFromSourceUrl(sourceUrl string) (string, error) { | ||
|
||
// Regexp for module name extraction. It assumes that the query string has already been stripped off. | ||
// Then we simply capture anything after the last slash, and before `.` or end of string. | ||
var moduleNameRegexp = regexp.MustCompile(`(?:.+/)(.+?)(?:\.|$)`) | ||
|
||
// strip off the query string if present | ||
sourceUrl = strings.Split(sourceUrl, "?")[0] | ||
|
||
matches := moduleNameRegexp.FindStringSubmatch(sourceUrl) | ||
|
||
// if regexp returns less/more than the full match + 1 capture group, then something went wrong with regex (invalid source string) | ||
if len(matches) != 2 { | ||
return "", errors.WithStackTrace(ErrorParsingModulePath{ModuleSourceUrl: sourceUrl}) | ||
} | ||
|
||
return matches[1], nil | ||
} | ||
|
||
// Custom error types | ||
type WrongNumberOfParams struct { | ||
Func string | ||
|
@@ -457,3 +523,21 @@ type TerragruntConfigNotFound struct { | |
func (err TerragruntConfigNotFound) Error() string { | ||
return fmt.Sprintf("Terragrunt config %s not found", err.Path) | ||
} | ||
|
||
type InvalidSourceUrl struct { | ||
ModulePath string | ||
ModuleSourceUrl string | ||
TerragruntSource string | ||
} | ||
|
||
func (err InvalidSourceUrl) Error() string { | ||
return fmt.Sprintf("The --terragrunt-source parameter is set to '%s', but the source URL in the module at '%s' is invalid: '%s'. Note that the module URL must have a double-slash to separate the repo URL from the path within the repo!", err.TerragruntSource, err.ModulePath, err.ModuleSourceUrl) | ||
} | ||
|
||
type ErrorParsingModulePath struct { | ||
ModuleSourceUrl string | ||
} | ||
|
||
func (err ErrorParsingModulePath) Error() string { | ||
return fmt.Sprintf("Unable to obtain the module path from the source URL '%s'. Ensure that the URL is in a supported format.", err.ModuleSourceUrl) | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -795,6 +795,44 @@ func TestReadTerragruntConfigLocals(t *testing.T) { | |
assert.Equal(t, localsMap["number_expression"].(float64), float64(42)) | ||
} | ||
|
||
func TestGetTerragruntSourceForModuleHappyPath(t *testing.T) { | ||
t.Parallel() | ||
|
||
testCases := []struct { | ||
config *TerragruntConfig | ||
source string | ||
expected string | ||
}{ | ||
{mockConfigWithSource(""), "", ""}, | ||
{mockConfigWithSource(""), "/source/modules", ""}, | ||
{mockConfigWithSource("git::[email protected]:acme/modules.git//foo/bar"), "/source/modules", "/source/modules//foo/bar"}, | ||
{mockConfigWithSource("git::[email protected]:acme/modules.git//foo/bar?ref=v0.0.1"), "/source/modules", "/source/modules//foo/bar"}, | ||
{mockConfigWithSource("git::[email protected]:acme/emr_cluster.git?ref=feature/fix_bugs"), "/source/modules", "/source/modules//emr_cluster"}, | ||
{mockConfigWithSource("git::ssh://[email protected]/OurOrg/some-module.git"), "/source/modules", "/source/modules//some-module"}, | ||
{mockConfigWithSource("github.com/hashicorp/example"), "/source/modules", "/source/modules//example"}, | ||
{mockConfigWithSource("github.com/hashicorp/example//subdir"), "/source/modules", "/source/modules//subdir"}, | ||
{mockConfigWithSource("[email protected]:hashicorp/example.git//subdir"), "/source/modules", "/source/modules//subdir"}, | ||
{mockConfigWithSource("./some/path//to/modulename"), "/source/modules", "/source/modules//to/modulename"}, | ||
} | ||
|
||
for _, testCase := range testCases { | ||
// The following is necessary to make sure testCase's values don't | ||
// get updated due to concurrency within the scope of t.Run(..) below | ||
testCase := testCase | ||
t.Run(fmt.Sprintf("%v-%s", *testCase.config.Terraform.Source, testCase.source), func(t *testing.T) { | ||
actual, err := GetTerragruntSourceForModule(testCase.source, "mock-for-test", testCase.config) | ||
require.NoError(t, err) | ||
assert.Equal(t, testCase.expected, actual) | ||
}) | ||
} | ||
} | ||
|
||
func mockConfigWithSource(sourceUrl string) *TerragruntConfig { | ||
cfg := TerragruntConfig{IsPartial: true} | ||
cfg.Terraform = &TerraformConfig{Source: &sourceUrl} | ||
return &cfg | ||
} | ||
|
||
// Return keys as a map so it is treated like a set, and order doesn't matter when comparing equivalence | ||
func getKeys(valueMap map[string]cty.Value) map[string]bool { | ||
keys := map[string]bool{} | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,7 +3,6 @@ package configstack | |
import ( | ||
"fmt" | ||
"path/filepath" | ||
"regexp" | ||
"sort" | ||
"strings" | ||
|
||
|
@@ -12,7 +11,6 @@ import ( | |
"github.com/gruntwork-io/terragrunt/options" | ||
"github.com/gruntwork-io/terragrunt/shell" | ||
"github.com/gruntwork-io/terragrunt/util" | ||
"github.com/hashicorp/go-getter" | ||
zglob "github.com/mattn/go-zglob" | ||
) | ||
|
||
|
@@ -260,7 +258,7 @@ func resolveTerraformModule(terragruntConfigPath string, terragruntOptions *opti | |
return nil, errors.WithStackTrace(ErrorProcessingModule{UnderlyingError: err, HowThisModuleWasFound: howThisModuleWasFound, ModulePath: terragruntConfigPath}) | ||
} | ||
|
||
terragruntSource, err := getTerragruntSourceForModule(modulePath, terragruntConfig, terragruntOptions) | ||
terragruntSource, err := config.GetTerragruntSourceForModule(terragruntOptions.Source, modulePath, terragruntConfig) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
@@ -295,69 +293,6 @@ func resolveTerraformModule(terragruntConfigPath string, terragruntOptions *opti | |
return &TerraformModule{Path: modulePath, Config: *terragruntConfig, TerragruntOptions: opts}, nil | ||
} | ||
|
||
// If one of the xxx-all commands is called with the --terragrunt-source parameter, then for each module, we need to | ||
// build its own --terragrunt-source parameter by doing the following: | ||
// | ||
// 1. Read the source URL from the Terragrunt configuration of each module | ||
// 2. Extract the path from that URL (the part after a double-slash) | ||
// 3. Append the path to the --terragrunt-source parameter | ||
// | ||
// Example: | ||
// | ||
// --terragrunt-source: /source/infrastructure-modules | ||
// source param in module's terragrunt.hcl: git::[email protected]:acme/infrastructure-modules.git//networking/vpc?ref=v0.0.1 | ||
// | ||
// This method will return: /source/infrastructure-modules//networking/vpc | ||
// | ||
func getTerragruntSourceForModule(modulePath string, moduleTerragruntConfig *config.TerragruntConfig, terragruntOptions *options.TerragruntOptions) (string, error) { | ||
if terragruntOptions.Source == "" || moduleTerragruntConfig.Terraform == nil || moduleTerragruntConfig.Terraform.Source == nil || *moduleTerragruntConfig.Terraform.Source == "" { | ||
return "", nil | ||
} | ||
|
||
// use go-getter to split the module source string into a valid URL and subdirectory (if // is present) | ||
moduleUrl, moduleSubdir := getter.SourceDirSubdir(*moduleTerragruntConfig.Terraform.Source) | ||
|
||
// if both URL and subdir are missing, something went terribly wrong | ||
if moduleUrl == "" && moduleSubdir == "" { | ||
return "", errors.WithStackTrace(InvalidSourceUrl{ModulePath: modulePath, ModuleSourceUrl: *moduleTerragruntConfig.Terraform.Source, TerragruntSource: terragruntOptions.Source}) | ||
} | ||
// if only subdir is missing, check if we can obtain a valid module name from the URL portion | ||
if moduleUrl != "" && moduleSubdir == "" { | ||
moduleSubdirFromUrl, err := getModulePathFromSourceUrl(moduleUrl) | ||
if err != nil { | ||
return moduleSubdirFromUrl, err | ||
} | ||
return util.JoinTerraformModulePath(terragruntOptions.Source, moduleSubdirFromUrl), nil | ||
} | ||
|
||
return util.JoinTerraformModulePath(terragruntOptions.Source, moduleSubdir), nil | ||
} | ||
|
||
// Parse sourceUrl not containing '//', and attempt to obtain a module path. | ||
// Example: | ||
// | ||
// sourceUrl = "git::ssh://[email protected]/OurOrg/module-name.git" | ||
// will return "module-name". | ||
|
||
func getModulePathFromSourceUrl(sourceUrl string) (string, error) { | ||
|
||
// Regexp for module name extraction. It assumes that the query string has already been stripped off. | ||
// Then we simply capture anything after the last slash, and before `.` or end of string. | ||
var moduleNameRegexp = regexp.MustCompile(`(?:.+/)(.+?)(?:\.|$)`) | ||
|
||
// strip off the query string if present | ||
sourceUrl = strings.Split(sourceUrl, "?")[0] | ||
|
||
matches := moduleNameRegexp.FindStringSubmatch(sourceUrl) | ||
|
||
// if regexp returns less/more than the full match + 1 capture group, then something went wrong with regex (invalid source string) | ||
if len(matches) != 2 { | ||
return "", errors.WithStackTrace(ErrorParsingModulePath{ModuleSourceUrl: sourceUrl}) | ||
} | ||
|
||
return matches[1], nil | ||
} | ||
|
||
// Look through the dependencies of the modules in the given map and resolve the "external" dependency paths listed in | ||
// each modules config (i.e. those dependencies not in the given list of Terragrunt config canonical file paths). | ||
// These external dependencies are outside of the current working directory, which means they may not be part of the | ||
|
@@ -554,24 +489,6 @@ func (err ErrorProcessingModule) Error() string { | |
return fmt.Sprintf("Error processing module at '%s'. How this module was found: %s. Underlying error: %v", err.ModulePath, err.HowThisModuleWasFound, err.UnderlyingError) | ||
} | ||
|
||
type InvalidSourceUrl struct { | ||
ModulePath string | ||
ModuleSourceUrl string | ||
TerragruntSource string | ||
} | ||
|
||
func (err InvalidSourceUrl) Error() string { | ||
return fmt.Sprintf("The --terragrunt-source parameter is set to '%s', but the source URL in the module at '%s' is invalid: '%s'. Note that the module URL must have a double-slash to separate the repo URL from the path within the repo!", err.TerragruntSource, err.ModulePath, err.ModuleSourceUrl) | ||
} | ||
|
||
type ErrorParsingModulePath struct { | ||
ModuleSourceUrl string | ||
} | ||
|
||
func (err ErrorParsingModulePath) Error() string { | ||
return fmt.Sprintf("Unable to obtain the module path from the source URL '%s'. Ensure that the URL is in a supported format.", err.ModuleSourceUrl) | ||
} | ||
|
||
type InfiniteRecursion struct { | ||
RecursionLevel int | ||
Modules map[string]*TerraformModule | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,6 @@ | ||
package configstack | ||
|
||
import ( | ||
"fmt" | ||
"os" | ||
"testing" | ||
|
||
|
@@ -771,53 +770,6 @@ func TestResolveTerraformModuleNoTerraformConfig(t *testing.T) { | |
assertModuleListsEqual(t, expected, actualModules) | ||
} | ||
|
||
func TestGetTerragruntSourceForModuleHappyPath(t *testing.T) { | ||
t.Parallel() | ||
|
||
testCases := []struct { | ||
config *config.TerragruntConfig | ||
opts *options.TerragruntOptions | ||
expected string | ||
}{ | ||
{mockConfigWithSource(""), mockOptionsWithSource(t, ""), ""}, | ||
{mockConfigWithSource(""), mockOptionsWithSource(t, "/source/modules"), ""}, | ||
{mockConfigWithSource("git::[email protected]:acme/modules.git//foo/bar"), mockOptionsWithSource(t, "/source/modules"), "/source/modules//foo/bar"}, | ||
{mockConfigWithSource("git::[email protected]:acme/modules.git//foo/bar?ref=v0.0.1"), mockOptionsWithSource(t, "/source/modules"), "/source/modules//foo/bar"}, | ||
{mockConfigWithSource("git::[email protected]:acme/emr_cluster.git?ref=feature/fix_bugs"), mockOptionsWithSource(t, "/source/modules"), "/source/modules//emr_cluster"}, | ||
{mockConfigWithSource("git::ssh://[email protected]/OurOrg/some-module.git"), mockOptionsWithSource(t, "/source/modules"), "/source/modules//some-module"}, | ||
{mockConfigWithSource("github.com/hashicorp/example"), mockOptionsWithSource(t, "/source/modules"), "/source/modules//example"}, | ||
{mockConfigWithSource("github.com/hashicorp/example//subdir"), mockOptionsWithSource(t, "/source/modules"), "/source/modules//subdir"}, | ||
{mockConfigWithSource("[email protected]:hashicorp/example.git//subdir"), mockOptionsWithSource(t, "/source/modules"), "/source/modules//subdir"}, | ||
{mockConfigWithSource("./some/path//to/modulename"), mockOptionsWithSource(t, "/source/modules"), "/source/modules//to/modulename"}, | ||
} | ||
|
||
for _, testCase := range testCases { | ||
// The following is necessary to make sure testCase's values don't | ||
// get updated due to concurrency within the scope of t.Run(..) below | ||
testCase := testCase | ||
t.Run(fmt.Sprintf("%v-%s", *testCase.config.Terraform.Source, testCase.opts.Source), func(t *testing.T) { | ||
actual, err := getTerragruntSourceForModule("mock-for-test", testCase.config, testCase.opts) | ||
require.NoError(t, err) | ||
assert.Equal(t, testCase.expected, actual) | ||
}) | ||
} | ||
} | ||
|
||
func mockOptionsWithSource(t *testing.T, sourceUrl string) *options.TerragruntOptions { | ||
opts, err := options.NewTerragruntOptionsForTest("mock-for-test.hcl") | ||
if err != nil { | ||
t.Fatalf("Error creating terragrunt options for test %v", err) | ||
} | ||
opts.Source = sourceUrl | ||
return opts | ||
} | ||
|
||
func mockConfigWithSource(sourceUrl string) *config.TerragruntConfig { | ||
cfg := config.TerragruntConfig{IsPartial: true} | ||
cfg.Terraform = &config.TerraformConfig{Source: &sourceUrl} | ||
return &cfg | ||
} | ||
|
||
func ptr(str string) *string { | ||
return &str | ||
} |
11 changes: 11 additions & 0 deletions
11
test/fixture-get-output/regression-1124/live/app/terragrunt.hcl
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
terraform { | ||
source = "../../modules//app" | ||
} | ||
|
||
dependency "dep" { | ||
config_path = "../dependency" | ||
} | ||
|
||
inputs = { | ||
foo = dependency.dep.outputs.foo | ||
} |
3 changes: 3 additions & 0 deletions
3
test/fixture-get-output/regression-1124/live/dependency/terragrunt.hcl
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
terraform { | ||
source = "../../modules//dependency" | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
variable "foo" {} | ||
output "foo" { value = var.foo } |
7 changes: 7 additions & 0 deletions
7
test/fixture-get-output/regression-1124/modules/dependency/main.tf
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
resource "random_string" "random" { | ||
length = 16 | ||
} | ||
|
||
output "foo" { | ||
value = random_string.random.result | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Just to make sure I understand here... If
terragrunt.hcl
has, say:And
module-b/terragrunt.hcl
contains:You're saying that if someone runs
terragrunt apply --terragrunt-source /local/path/to/modules
, then we are overriding thesource
parameter in both theterragrunt.hcl
ofmodule-a
and ofmodule-b
?If so, then this highlights two issues with the
--terragrunt-source
design:terragrunt apply --terragrunt-source /foo/bar
in folderyyy
, as of now, the only thing it does is override thesource
URL inyyy/terragrunt.hcl
to the exact value/foo/bar
. However, if you runterragrunt apply-all --terragrunt-source /foo/bar
in folderyyy
, it overrides thesource
value of each child module withinyyy/sub/path
to something like/foo/bar/sub/path
. The inconsistency between these two behaviors is being highlighted in this PR. Would it make sense to always use the latter behavior? That is--terragrunt-source
should always be set to the root of your modules repo?--terragrunt-source
: it assumes all your modules are coming from the same root repo. You won't be able to do anapply-all
if you're pulling from multiple repos (e.g., your own infra modules, Terraform Registry, Gruntwork repos, etc).Perhaps a better solution to this is to support some sort of mapping?
The
--terragrunt-source-map
param would replace anysource
URL that starts withsource
to instead start withdest
.For example:
The above would replace
source = "github.com/org/modules.git//xxx"
with/local/path/to/modules//xxx
regardless of whether you were runningapply
, orapply-all
, or using adependency
. This would give us a consistent behavior across all command and support mapping multiple repos by specifying the param multiple times:We could also consider allowing users to specify this mapping in a file:
Where
mapping.hcl
contains:Note, I realize the above is a bigger change, so it could also be filed as an issue and done in a separate PR 😁
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.
Filed #1138
By the way, are you suggesting we close this PR in favor of solving for the mapping case, or are you ok with the band-aid fix here?
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.
Thanks! I just updated it with the more detailed description above.
Your call! I'm OK with either approach. Fixing #1138 would be great, but definitely isn't a high priority, so a band-aid fix for now would be fine too. #1138 might make for a good trial project in the future, too.
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.
Sounds good. +1 to #1138 being a good trial project. Thanks for the sanity check!