Workflow bundling for runtime #4
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,28 @@ | ||||||||||||||
| package project_bundler | ||||||||||||||
|
|
||||||||||||||
| type ( | ||||||||||||||
| ProjectConfiguration struct { | ||||||||||||||
| Version string `json:"version"` | ||||||||||||||
| RootDir string `json:"root_dir"` | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| KindeWorkflow struct { | ||||||||||||||
| WorkflowRootDirectory string `json:"workflow_root_directory"` | ||||||||||||||
| } | ||||||||||||||
| KindeWorkflows struct { | ||||||||||||||
| Workflows []KindeWorkflow `json:"workflows"` | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| KindeEnvironment struct { | ||||||||||||||
| Workflows KindeWorkflows `json:"workflows"` | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| KindeProject struct { | ||||||||||||||
| Configuration ProjectConfiguration `json:"configuration"` | ||||||||||||||
| Environment KindeEnvironment `json:"environment"` | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| ProjectBundler interface { | ||||||||||||||
| Discover() KindeProject | ||||||||||||||
| } | ||||||||||||||
|
There was a problem hiding this comment. 🛠️ Refactor suggestion Consider enhancing the ProjectBundler interface for better error handling and context support. While the
Here's a suggested improvement: type ProjectBundler interface {
- Discover() KindeProject
+ DiscoverProject(ctx context.Context) (KindeProject, error)
}This change would allow for more robust error handling and support for cancellation or timeouts via the context. Committable suggestion
Suggested change
|
||||||||||||||
| ) | ||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| package project_bundler | ||
|
|
||
| import "testing" | ||
|
|
||
| func Test_ProjectBunler(t *testing.T) { | ||
| } | ||
|
There was a problem hiding this comment. Fix typo in function name and implement test logic. There are two issues with the test function:
Please apply the following changes:
-func Test_ProjectBunler(t *testing.T) {
+func Test_ProjectBundler(t *testing.T) {
func Test_ProjectBundler(t *testing.T) {
// TODO: Implement test cases for ProjectBundler
t.Run("Test case description", func(t *testing.T) {
// Add assertions here
})
}Would you like assistance in designing and implementing specific test cases for the ProjectBundler? |
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -3,7 +3,7 @@ package runtime_registry | |||||||||||||||||||||||||
| import ( | ||||||||||||||||||||||||||
| "context" | ||||||||||||||||||||||||||
| "crypto/sha256" | ||||||||||||||||||||||||||
| "encoding/base32" | ||||||||||||||||||||||||||
| "fmt" | ||||||||||||||||||||||||||
| "time" | ||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||
|
|
@@ -21,11 +21,6 @@ type ( | |||||||||||||||||||||||||
| Arguments []interface{} | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| CodeDescriptor struct { | ||||||||||||||||||||||||||
| Source []byte `json:"source"` | ||||||||||||||||||||||||||
| SourceType SourceContentType `json:"source_type"` | ||||||||||||||||||||||||||
|
|
@@ -50,21 +45,35 @@ type ( | |||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| WorkflowDescriptor struct { | ||||||||||||||||||||||||||
| ProcessedSource CodeDescriptor `json:"processed_source"` | ||||||||||||||||||||||||||
| Bindings Bindings `json:"bindings"` | ||||||||||||||||||||||||||
| Limits RuntimeLimits `json:"runtime_limits"` | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| ExecutionResult interface { | ||||||||||||||||||||||||||
| GetExitResult() interface{} | ||||||||||||||||||||||||||
| GetConsoleLog() []interface{} | ||||||||||||||||||||||||||
| GetConsoleError() []interface{} | ||||||||||||||||||||||||||
| GetContext() map[string]interface{} | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
Comment on lines
+53
to
58
There was a problem hiding this comment. 💡 Codebase verification Issues Found
Analysis chainApprove renaming of Result to ExecutionResult The renaming of To ensure all usages of this interface have been updated, please run the following script: Further Verification Needed Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Verify all occurrences of the old interface name have been updated.
# Test: Search for any remaining instances of the old interface name. Expect: No results.
rg --type go '\bResult\b' --glob '!registry/runtimeRegistry.go'
# Test: Confirm the new interface name is used correctly. Expect: Occurrences of ExecutionResult.
rg --type go '\bExecutionResult\b'
Length of output: 412 Script: #!/bin/bash
# Description: Verify all occurrences of the old interface name have been updated in both source and test files.
# Test: Search for any remaining instances of the old interface name. Expect: No results.
rg --type go '\bResult\b' --glob '!registry/runtimeRegistry.go'
# Test: Confirm the new interface name is used correctly. Expect: Occurrences of ExecutionResult.
rg --type go '\bExecutionResult\b'
Length of output: 985 Script: #!/bin/bash
# Description: Ensure that Runner interface tests include the new Introspect method.
# Test: Search for tests of the Runner interface to confirm Introspect method is included.
rg --type go 'func TestRunner.*Introspect' --glob '*_test.go'
Length of output: 63 |
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
|
Comment on lines
+53
to
59
There was a problem hiding this comment. 🛠️ Refactor suggestion Refactor accessor method names to follow Go conventions In Go, it's idiomatic to omit the
Apply this diff to update the method names: ExecutionResult interface {
- GetExitResult() interface{}
- GetConsoleLog() []interface{}
- GetConsoleError() []interface{}
- GetContext() map[string]interface{}
+ ExitResult() interface{}
+ ConsoleLog() []interface{}
+ ConsoleError() []interface{}
+ Context() map[string]interface{}
}Committable suggestion
Suggested change
|
||||||||||||||||||||||||||
| IntrospectedExport interface { | ||||||||||||||||||||||||||
| HasExport() bool | ||||||||||||||||||||||||||
| Value() interface{} | ||||||||||||||||||||||||||
| ValueAsMap() map[string]interface{} | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| IntrospectionResult interface { | ||||||||||||||||||||||||||
| GetExport(string) IntrospectedExport | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
Comment on lines
+66
to
+68
There was a problem hiding this comment. 🛠️ Refactor suggestion Rename 'GetExport' to 'Export' in 'IntrospectionResult' interface Following Go's naming conventions, consider renaming the Apply this diff to rename the method: IntrospectionResult interface {
- GetExport(string) IntrospectedExport
+ Export(string) IntrospectedExport
}Committable suggestion
Suggested change
|
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| InstrospectionOptions struct { | ||||||||||||||||||||||||||
| Exports []string | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
There was a problem hiding this comment. Approve new struct, but correct the spelling The new Please correct the spelling as follows: -InstrospectionOptions struct {
+IntrospectionOptions struct {
Exports []string
}Committable suggestion
Suggested change
|
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| Runner interface { | ||||||||||||||||||||||||||
| Execute(ctx context.Context, workflow WorkflowDescriptor, startOptions StartOptions) (ExecutionResult, error) | ||||||||||||||||||||||||||
| Introspect(ctx context.Context, workflow WorkflowDescriptor, options InstrospectionOptions) (IntrospectionResult, error) | ||||||||||||||||||||||||||
|
There was a problem hiding this comment. Approve interface changes, but correct the spelling The updates to the However, there's a spelling error in the Runner interface {
Execute(ctx context.Context, workflow WorkflowDescriptor, startOptions StartOptions) (ExecutionResult, error)
- Introspect(ctx context.Context, workflow WorkflowDescriptor, options InstrospectionOptions) (IntrospectionResult, error)
+ Introspect(ctx context.Context, workflow WorkflowDescriptor, options IntrospectionOptions) (IntrospectionResult, error)
}Committable suggestion
Suggested change
|
||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
|
|
@@ -85,9 +94,8 @@ func ResolveRuntime(name string) (Runner, error) { | |||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| // Returns a hash of the workflow descriptor | ||||||||||||||||||||||||||
| func (wd *WorkflowDescriptor) GetHash() string { | ||||||||||||||||||||||||||
| sha := sha256.New() | ||||||||||||||||||||||||||
| sha.Write([]byte(wd.ProcessedSource.Source)) | ||||||||||||||||||||||||||
| result := base32.StdEncoding.EncodeToString(sha.Sum(nil)) | ||||||||||||||||||||||||||
| return fmt.Sprintf("%v", result) | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -51,5 +51,4 @@ func Test_GojaRuntime(t *testing.T) { | |
| assert := assert.New(t) | ||
| assert.Nil(err) | ||
| assert.Equal("fetch response", fmt.Sprintf("%v", result.GetExitResult())) | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| { | ||
| "version": "2024-12-09", | ||
| "rootDir": "kindeSrc" | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| export const hello = 'hello'; |
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.
Consider safer type assertion in ValueAsMap method
The
ValueAsMapmethod uses a type assertion that could potentially panic if thevalueis not of typemap[string]interface{}.Consider using a type assertion with the
okidiom to safely handle cases where the type assertion may fail:func (i introspectedExport) ValueAsMap() map[string]interface{} { if i.value == nil { return map[string]interface{}{} } - return i.value.(map[string]interface{}) + if m, ok := i.value.(map[string]interface{}); ok { + return m + } + return map[string]interface{}{} }Committable suggestion