Go fs #219

jhsinger-klotho · 2023-02-08T20:18:22Z

• Does any part of it require special attention?
• Does it relate to or fix any issue?

This is the parsing and transformation side of https://github.com/klothoplatform/klotho-history/issues/466.

We look for the fileblob and transform that to the s3 defintition found in https://gocloud.dev/howto/blob/.

Going to do the IAC side after this so it doesnt grow too large

Standard checks

Unit tests: Any special considerations? Added for query and transform as well as some of the helper functions
Docs: Do we need to update any docs, internal or public? Not yet
Backwards compatibility: Will this break existing apps? If so, what would be the extra work required to keep them working? yes, new feature

jhsinger-klotho · 2023-02-09T01:32:19Z

Need to update this to be in sync with updates in the imports pr

DavidSeptimus-Klotho · 2023-02-09T13:39:30Z

pkg/lang/golang/arguments.go

+}
+
+func (a *Argument) IsString() bool {
+	return a.Type == "interpreted_string_literal"


Is the intent here not to match raw_string_literal (using backticks - ``)?

ah is that a valid go node type with the only difference being `` instead of ""? If so, i definitely need to add that

DavidSeptimus-Klotho · 2023-02-09T13:49:07Z

pkg/lang/golang/plugin_fs.go

+		for _, f := range unit.Files() {
+			goSource, ok := Language.ID.CastFile(f)
+			if !ok {
+				continue
+			}


nit: could be simplified with

for _, goSource := range unit.FilesOfLang(goLang) { ... }

will update

DavidSeptimus-Klotho · 2023-02-09T13:52:39Z

pkg/lang/golang/plugin_fs.go

+	}
+
+	args[0].Content = "nil"
+	args[1].Content = fmt.Sprintf(`os.Getenv("%s")`, fsEnvVar.Name)


Do we check to make sure os is imported anywhere?

good callout, i need to make a bunch of updates but i definitely missed this originally

DavidSeptimus-Klotho · 2023-02-09T13:55:40Z

pkg/lang/golang/plugin_fs.go

+	args[0].Content = "nil"
+	args[1].Content = fmt.Sprintf(`os.Getenv("%s")`, fsEnvVar.Name)
+
+	newNodeContent := strings.Replace(result.args.Content(), result.args.Content(), ArgumentListToString(args), 1)


I'm confused here. Won't the outcome of strings.Replace() always be to replace the entirety of the original string with the value of ArgumentListToString(args)?

yeah so were replacing all of the args node which is (var1, var2) with our new args (newVar1, newVar2)

What David's saying is that this is the same as

newNodeContent := ArgumentListToString(args)

oh yeah i see, good point let me update

DavidSeptimus-Klotho · 2023-02-09T14:01:21Z

pkg/lang/golang/queries/gocloud/file_bucket.scm

@@ -0,0 +1,18 @@
+(short_var_declaration


no long var declarations? 😢

i guess not, go is a simpleton

This probably won't handle:

var bucket, err = fileblob.OpenBucket(myDir, myConnector)

(presumably the (long) var_declaration)

or

var bucket, err bucket, err = fileblob.OpenBucket(myDir, myConnector)

let me update the query, should be able to make it work without having to change any of the logic i believe

gordon-klotho · 2023-02-09T17:48:41Z

pkg/lang/golang/runtime_test.go

+func (n NoopRuntime) GetFsImports() []string {
+	return []string{
+		"gocloud.dev/blob",
+		"_ \"gocloud.dev/blob/s3blob\"",


Suggested change

"_ \"gocloud.dev/blob/s3blob\"",

`_ "gocloud.dev/blob/s3blob"`,

nit

yup, caused updated tests to break as well so this should be fixed. Was leftover from the go imports pr that was updated

gordon-klotho · 2023-02-09T17:51:11Z

pkg/lang/golang/imports.go

@@ -64,14 +74,23 @@ func UpdateImportsInFile(f *core.SourceFile, importsToAdd []string, importsToRem

 	// Determine which imports already exist and which we need to add
 	for _, i := range importsToAdd {
+		importChunks := strings.Split(i, "\"")


Suggested change

importChunks := strings.Split(i, "\"")

importChunks := strings.Split(i, `"`)

nit

this should be removed now since we take in imports

gordon-klotho · 2023-02-09T17:52:03Z

pkg/lang/golang/aws_runtime/aws.go

+func (r *AwsRuntime) GetFsImports() []string {
+	return []string{
+		"gocloud.dev/blob",
+		"_ \"gocloud.dev/blob/s3blob\"",


Suggested change

"_ \"gocloud.dev/blob/s3blob\"",

`_ "gocloud.dev/blob/s3blob"`,

nit

same as above

gordon-klotho · 2023-02-10T02:17:20Z

pkg/lang/golang/queries/gocloud/file_bucket.scm

@@ -0,0 +1,18 @@
+(short_var_declaration


This probably won't handle:

var bucket, err = fileblob.OpenBucket(myDir, myConnector)

(presumably the (long) var_declaration)

or

var bucket, err bucket, err = fileblob.OpenBucket(myDir, myConnector)

gordon-klotho · 2023-02-10T02:18:56Z

pkg/lang/golang/queries/find_args.scm

@@ -0,0 +1,4 @@
+; Finds arguments in an arguments_list
+(argument_list [


What's the [ for?

so i copied this over from python, must have just been leftover, will remove

…can be set

github-actions · 2023-02-10T17:24:23Z

Package	Line Rate	Health
github.com/klothoplatform/klotho/pkg/analytics	2%	❌
github.com/klothoplatform/klotho/pkg/annotation	23%	❌
github.com/klothoplatform/klotho/pkg/cli	4%	❌
github.com/klothoplatform/klotho/pkg/core	21%	❌
github.com/klothoplatform/klotho/pkg/env_var	82%	✔
github.com/klothoplatform/klotho/pkg/exec_unit	54%	➖
github.com/klothoplatform/klotho/pkg/infra/kubernetes	59%	➖
github.com/klothoplatform/klotho/pkg/infra/kubernetes/helm	39%	❌
github.com/klothoplatform/klotho/pkg/input	63%	➖
github.com/klothoplatform/klotho/pkg/lang	38%	❌
github.com/klothoplatform/klotho/pkg/lang/dockerfile	0%	❌
github.com/klothoplatform/klotho/pkg/lang/golang	33%	❌
github.com/klothoplatform/klotho/pkg/lang/javascript	47%	❌
github.com/klothoplatform/klotho/pkg/lang/python	61%	➖
github.com/klothoplatform/klotho/pkg/lang/yaml	0%	❌
github.com/klothoplatform/klotho/pkg/logging	6%	❌
github.com/klothoplatform/klotho/pkg/multierr	95%	✔
github.com/klothoplatform/klotho/pkg/provider/aws	59%	➖
github.com/klothoplatform/klotho/pkg/runtime	78%	✔
github.com/klothoplatform/klotho/pkg/static_unit	33%	❌
github.com/klothoplatform/klotho/pkg/validation	73%	➖
github.com/klothoplatform/klotho/pkg/yaml_util	79%	✔
Summary	41% (3952 / 9632)	❌

ghost · 2023-02-15T15:40:34Z

pkg/lang/golang/arguments.go

+}
+
+// GetArguements is passed a tree-sitter node, which is of type argument_list, and returns a list of in order Arguments
+func GetArguements(args *sitter.Node) []Argument {


typo:

- GetArguements + GetArguments

ghost · 2023-02-15T16:01:43Z

pkg/lang/golang/arguments.go

+	nextMatch := doQuery(args, findArgs)
+	for {
+		match, found := nextMatch()
+		if !found {
+			break
+		}
+
+		arg := match["arg"]
+		if arg == nil {
+			continue
+		}
+
+		arguments = append(arguments, Argument{Content: arg.Content(), Type: arg.Type()})
+	}


Totally optional, but you and also just do:

selection := query.Select(query.Exec(language, args, findArgs), query.ParamNamed("arg")) for _, arg := range query.Collect(selection) { arguments = append(arguments, Argument{Content: arg.Content(), Type: arg.Type()}) }

ghost · 2023-02-15T16:04:04Z

pkg/lang/golang/imports.go

@@ -47,6 +47,16 @@ func GetImportsInFile(f *core.SourceFile) []Import {
 	return imports
 }

+func GetNamedImportInFile(f *core.SourceFile, namedImport string) Import {
+	imports := GetImportsInFile(f)
+	for _, i := range imports {


minor-ish: I pretty much always assume i is an index in a for, so this threw me off at first. Maybe imp?

yeah because i have changes in other prs i will go ahead and update in a future pr. Same with the above comments

DavidSeptimus-Klotho reviewed Feb 9, 2023

View reviewed changes

gordon-klotho reviewed Feb 10, 2023

View reviewed changes

Base automatically changed from go_imports to main February 10, 2023 15:01

Jordan Singer added 4 commits February 10, 2023 07:10

initial imports support for golang

c499ef0

fs go plugin

3113a41

remove membucket query

21909d4

update with new imports code and find more syntax for how the bucket …

91f3dd9

…can be set

jhsinger-klotho force-pushed the go_fs branch from 378c393 to 91f3dd9 Compare February 10, 2023 17:18

remove unneeded code

7e3b14d

jhsinger-klotho mentioned this pull request Feb 13, 2023

Go fs iac #224

Merged

ghost approved these changes Feb 15, 2023

View reviewed changes

jhsinger-klotho merged commit 330a3a7 into main Feb 15, 2023

jhsinger-klotho deleted the go_fs branch February 15, 2023 16:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Go fs #219

Go fs #219

jhsinger-klotho commented Feb 8, 2023

jhsinger-klotho commented Feb 9, 2023

DavidSeptimus-Klotho Feb 9, 2023

jhsinger-klotho Feb 10, 2023

DavidSeptimus-Klotho Feb 9, 2023

jhsinger-klotho Feb 10, 2023

DavidSeptimus-Klotho Feb 9, 2023

jhsinger-klotho Feb 10, 2023

DavidSeptimus-Klotho Feb 9, 2023

jhsinger-klotho Feb 10, 2023

gordon-klotho Feb 10, 2023

jhsinger-klotho Feb 10, 2023

DavidSeptimus-Klotho Feb 9, 2023

jhsinger-klotho Feb 10, 2023

gordon-klotho Feb 10, 2023

jhsinger-klotho Feb 10, 2023

gordon-klotho Feb 9, 2023

jhsinger-klotho Feb 10, 2023

gordon-klotho Feb 9, 2023

jhsinger-klotho Feb 10, 2023

gordon-klotho Feb 9, 2023

jhsinger-klotho Feb 10, 2023

gordon-klotho Feb 10, 2023

gordon-klotho Feb 10, 2023

jhsinger-klotho Feb 10, 2023

github-actions bot commented Feb 10, 2023

ghost Feb 15, 2023

ghost Feb 15, 2023

ghost Feb 15, 2023

jhsinger-klotho Feb 15, 2023

	"_ \"gocloud.dev/blob/s3blob\"",
	`_ "gocloud.dev/blob/s3blob"`,

	importChunks := strings.Split(i, "\"")
	importChunks := strings.Split(i, `"`)

		@@ -0,0 +1,4 @@
		; Finds arguments in an arguments_list
		(argument_list [

Go fs #219

Go fs #219

Conversation

jhsinger-klotho commented Feb 8, 2023

Standard checks

jhsinger-klotho commented Feb 9, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Feb 10, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment