Skip to content
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

feat: separate out language logic from moduleconfig #3016

Merged
merged 16 commits into from
Oct 8, 2024
Merged
55 changes: 44 additions & 11 deletions backend/controller/admin/local_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,14 @@ package admin
import (
"context"
"fmt"
"path/filepath"

"github.com/alecthomas/types/either"
"github.com/alecthomas/types/optional"

"github.com/TBD54566975/ftl/internal/buildengine/languageplugin"
cf "github.com/TBD54566975/ftl/internal/configuration"
"github.com/TBD54566975/ftl/internal/configuration/manager"
"github.com/TBD54566975/ftl/internal/errors"
"github.com/TBD54566975/ftl/internal/projectconfig"
"github.com/TBD54566975/ftl/internal/schema"
"github.com/TBD54566975/ftl/internal/watch"
Expand Down Expand Up @@ -45,19 +47,50 @@ func (s *diskSchemaRetriever) GetActiveSchema(ctx context.Context) (*schema.Sche
return nil, fmt.Errorf("could not discover modules: %w", err)
}

sch := &schema.Schema{}
moduleSchemas := make(chan either.Either[*schema.Module, error], 32)
defer close(moduleSchemas)

for _, m := range modules {
config := m.Abs()
schemaPath := config.Schema()
if r, ok := s.deployRoot.Get(); ok {
schemaPath = filepath.Join(r, m.Module, m.DeployDir, m.Schema())
}
go func() {
// Loading a plugin can be expensive. Is there a better way?
plugin, err := languageplugin.New(ctx, m.Language)
Copy link
Collaborator Author

@matt2e matt2e Oct 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change will eventually (once language plugins operate over gRPC) lead to a slowdown in this logic because now we need to create a language plugin per module just so we can figure out where the schema is saved.

I'm not sure of a good solution to this, other than having a central place FTL saves schemas (similar to the central /.ftl/ external modules) so that having a ModuleConfig is not readed to read the saved schema for a module.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not specifically about the slowdown, but the schema won't be saved right? It will be returned over gRPC?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Plugins don't save the schema, it gets sent over gRPC. What is unresolved at the moment though is how FTL deals with the schema after that. This admin logic is something that breaks if the schema doesn't get saved at all. For now FTL itself is saving the schema just so this and the current deploy logic works and I'll be coming back to this admin stuff a bit later. I've added a section on deployment to the things to do in this main issue here: #2452

if err != nil {
moduleSchemas <- either.RightOf[*schema.Module](fmt.Errorf("could not load plugin for %s: %w", m.Module, err))
}
defer plugin.Kill(ctx) // nolint:errcheck

customDefaults, err := plugin.ModuleConfigDefaults(ctx, m.Dir)
if err != nil {
moduleSchemas <- either.RightOf[*schema.Module](fmt.Errorf("could not get module config defaults for %s: %w", m.Module, err))
}

module, err := schema.ModuleFromProtoFile(schemaPath)
if err != nil {
return nil, fmt.Errorf("could not load module schema: %w", err)
config, err := m.FillDefaultsAndValidate(customDefaults)
if err != nil {
moduleSchemas <- either.RightOf[*schema.Module](fmt.Errorf("could not validate module config for %s: %w", m.Module, err))
}
module, err := schema.ModuleFromProtoFile(config.Abs().Schema())
if err != nil {
moduleSchemas <- either.RightOf[*schema.Module](fmt.Errorf("could not load module schema: %w", err))
return
}
moduleSchemas <- either.LeftOf[error](module)
}()
}
sch := &schema.Schema{}
errs := []error{}
for range len(modules) {
result := <-moduleSchemas
switch result := result.(type) {
case either.Left[*schema.Module, error]:
sch.Upsert(result.Get())
case either.Right[*schema.Module, error]:
errs = append(errs, result.Get())
default:
panic(fmt.Sprintf("unexpected type %T", result))
}
sch.Upsert(module)
}
if len(errs) > 0 {
return nil, errors.Join(errs...)
}
return sch, nil
}
3 changes: 3 additions & 0 deletions examples/go/time/ftl.toml
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,6 @@ language = "go"

[go.replace]
"github.com/TBD54566975/ftl" = "../.."

[go]
package = "time"
8 changes: 3 additions & 5 deletions frontend/cli/cmd_new.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import (
"github.com/alecthomas/kong"

"github.com/TBD54566975/ftl/internal"
"github.com/TBD54566975/ftl/internal/buildengine"
"github.com/TBD54566975/ftl/internal/buildengine/languageplugin"
"github.com/TBD54566975/ftl/internal/log"
"github.com/TBD54566975/ftl/internal/moduleconfig"
"github.com/TBD54566975/ftl/internal/projectconfig"
Expand Down Expand Up @@ -50,9 +50,7 @@ func prepareNewCmd(ctx context.Context, k *kong.Kong, args []string) error {
return fmt.Errorf("could not find new command")
}

plugin, err := buildengine.PluginFromConfig(ctx, moduleconfig.ModuleConfig{
Language: language,
}, "")
plugin, err := languageplugin.New(ctx, language)
if err != nil {
return fmt.Errorf("could not create plugin for %v: %w", language, err)
}
Expand Down Expand Up @@ -106,7 +104,7 @@ func (i newCmd) Run(ctx context.Context, ktctx *kong.Context, config projectconf
flags[f.Name] = flagValue
}

plugin, err := buildengine.PluginFromConfig(ctx, moduleConfig, config.Root())
plugin, err := languageplugin.New(ctx, i.Language)
if err != nil {
return err
}
Expand Down
62 changes: 48 additions & 14 deletions frontend/cli/cmd_schema_diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (

"connectrpc.com/connect"
"github.com/alecthomas/chroma/v2/quick"
"github.com/alecthomas/types/either"
"github.com/hexops/gotextdiff"
"github.com/hexops/gotextdiff/myers"
"github.com/hexops/gotextdiff/span"
Expand All @@ -17,6 +18,7 @@ import (
ftlv1 "github.com/TBD54566975/ftl/backend/protos/xyz/block/ftl/v1"
"github.com/TBD54566975/ftl/backend/protos/xyz/block/ftl/v1/ftlv1connect"
schemapb "github.com/TBD54566975/ftl/backend/protos/xyz/block/ftl/v1/schema"
"github.com/TBD54566975/ftl/internal/buildengine/languageplugin"
"github.com/TBD54566975/ftl/internal/log"
"github.com/TBD54566975/ftl/internal/projectconfig"
"github.com/TBD54566975/ftl/internal/rpc"
Expand Down Expand Up @@ -89,27 +91,59 @@ func (d *schemaDiffCmd) Run(ctx context.Context, currentURL *url.URL, projConfig
}

func localSchema(ctx context.Context, projectConfig projectconfig.Config) (*schema.Schema, error) {
pb := &schema.Schema{}
found := false
tried := ""
errs := []error{}
modules, err := watch.DiscoverModules(ctx, projectConfig.AbsModuleDirs())
if err != nil {
return nil, fmt.Errorf("failed to discover modules %w", err)
}
for _, moduleSettings := range modules {
mod, err := schema.ModuleFromProtoFile(moduleSettings.Abs().Schema())
if err != nil {
tried += fmt.Sprintf(" failed to read schema file %s; did you run ftl build?", moduleSettings.Abs().Schema())
} else {
found = true
pb.Modules = append(pb.Modules, mod)
}

moduleSchemas := make(chan either.Either[*schema.Module, error], len(modules))
defer close(moduleSchemas)

for _, m := range modules {
go func() {
// Loading a plugin can be expensive. Is there a better way?
plugin, err := languageplugin.New(ctx, m.Language)
if err != nil {
moduleSchemas <- either.RightOf[*schema.Module](err)
}
defer plugin.Kill(ctx) // nolint:errcheck

customDefaults, err := plugin.ModuleConfigDefaults(ctx, m.Dir)
if err != nil {
moduleSchemas <- either.RightOf[*schema.Module](err)
}

config, err := m.FillDefaultsAndValidate(customDefaults)
if err != nil {
moduleSchemas <- either.RightOf[*schema.Module](err)
}
module, err := schema.ModuleFromProtoFile(config.Abs().Schema())
if err != nil {
moduleSchemas <- either.RightOf[*schema.Module](err)
return
}
moduleSchemas <- either.LeftOf[error](module)
}()
}
sch := &schema.Schema{}
for range len(modules) {
result := <-moduleSchemas
switch result := result.(type) {
case either.Left[*schema.Module, error]:
sch.Upsert(result.Get())
case either.Right[*schema.Module, error]:
errs = append(errs, result.Get())
default:
panic(fmt.Sprintf("unexpected type %T", result))

}
}
if !found {
return nil, errors.New(tried)
// we want schema even if there are errors as long as we have some modules
if len(sch.Modules) == 0 && len(errs) > 0 {
return nil, fmt.Errorf("failed to read schema, possibly due to not building: %w", errors.Join(errs...))
}
return pb, nil
return sch, nil
}

func schemaForURL(ctx context.Context, url url.URL) (*schema.Schema, error) {
Expand Down
19 changes: 9 additions & 10 deletions go-runtime/compile/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ func buildDir(moduleDir string) string {
}

// Build the given module.
func Build(ctx context.Context, projectRootDir, moduleDir string, sch *schema.Schema, filesTransaction ModifyFilesTransaction, buildEnv []string, devMode bool) (err error) {
func Build(ctx context.Context, projectRootDir, moduleDir string, config moduleconfig.AbsModuleConfig, sch *schema.Schema, filesTransaction ModifyFilesTransaction, buildEnv []string, devMode bool) (err error) {
if err := filesTransaction.Begin(); err != nil {
return err
}
Expand Down Expand Up @@ -296,11 +296,6 @@ func Build(ctx context.Context, projectRootDir, moduleDir string, sch *schema.Sc
projectName = pc.Name
}

config, err := moduleconfig.LoadModuleConfig(moduleDir)
if err != nil {
return fmt.Errorf("failed to load module config: %w", err)
}

logger := log.FromContext(ctx)
funcs := maps.Clone(scaffoldFuncs)

Expand Down Expand Up @@ -1159,7 +1154,7 @@ func shouldUpdateVersion(goModfile *modfile.File) bool {
return true
}

func writeSchema(config moduleconfig.ModuleConfig, module *schema.Module) error {
func writeSchema(config moduleconfig.AbsModuleConfig, module *schema.Module) error {
modulepb := module.ToProto().(*schemapb.Module) //nolint:forcetypeassert
// If user has overridden GOOS and GOARCH we want to use those values.
goos, ok := os.LookupEnv("GOOS")
Expand All @@ -1181,19 +1176,23 @@ func writeSchema(config moduleconfig.ModuleConfig, module *schema.Module) error
if err != nil {
return fmt.Errorf("failed to marshal schema: %w", err)
}
err = os.WriteFile(config.Abs().Schema(), schemaBytes, 0600)
err = os.WriteFile(config.Schema(), schemaBytes, 0600)
if err != nil {
return fmt.Errorf("could not write schema: %w", err)
}
return nil
}

func writeSchemaErrors(config moduleconfig.ModuleConfig, errors []builderrors.Error) error {
func writeSchemaErrors(config moduleconfig.AbsModuleConfig, errors []builderrors.Error) error {
elBytes, err := proto.Marshal(languagepb.ErrorsToProto(errors))
if err != nil {
return fmt.Errorf("failed to marshal errors: %w", err)
}
return os.WriteFile(config.Abs().Errors, elBytes, 0600)
err = os.WriteFile(config.Errors, elBytes, 0600)
if err != nil {
return fmt.Errorf("could not write build errors: %w", err)
}
return nil
}

// returns the import path and directory name for a Go type
Expand Down
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ require (
github.com/docker/docker v27.3.1+incompatible
github.com/docker/go-connections v0.5.0
github.com/go-logr/logr v1.4.2
github.com/go-viper/mapstructure/v2 v2.2.1
github.com/google/uuid v1.6.0
github.com/hashicorp/cronexpr v1.1.2
github.com/jackc/pgerrcode v0.0.0-20240316143900-6e2875d9b438
Expand Down
2 changes: 2 additions & 0 deletions go.sum

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

13 changes: 7 additions & 6 deletions internal/buildengine/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/alecthomas/types/either"
"google.golang.org/protobuf/proto"

"github.com/TBD54566975/ftl/internal/buildengine/languageplugin"
"github.com/TBD54566975/ftl/internal/builderrors"
"github.com/TBD54566975/ftl/internal/errors"
"github.com/TBD54566975/ftl/internal/log"
Expand All @@ -19,29 +20,29 @@ import (
// Build a module in the given directory given the schema and module config.
//
// A lock file is used to ensure that only one build is running at a time.
func build(ctx context.Context, plugin LanguagePlugin, projectRootDir string, sch *schema.Schema, config moduleconfig.ModuleConfig, buildEnv []string, devMode bool) (*schema.Module, error) {
func build(ctx context.Context, plugin languageplugin.LanguagePlugin, projectRootDir string, sch *schema.Schema, config moduleconfig.ModuleConfig, buildEnv []string, devMode bool) (*schema.Module, error) {
logger := log.FromContext(ctx).Module(config.Module).Scope("build")
ctx = log.ContextWithLogger(ctx, logger)

logger.Infof("Building module")

result, err := plugin.Build(ctx, projectRootDir, config, sch, buildEnv, devMode)
if err != nil {
return handleBuildResult(ctx, config, either.RightOf[BuildResult](err))
return handleBuildResult(ctx, config, either.RightOf[languageplugin.BuildResult](err))
}
return handleBuildResult(ctx, config, either.LeftOf[error](result))
}

// handleBuildResult processes the result of a build
func handleBuildResult(ctx context.Context, c moduleconfig.ModuleConfig, eitherResult either.Either[BuildResult, error]) (*schema.Module, error) {
func handleBuildResult(ctx context.Context, c moduleconfig.ModuleConfig, eitherResult either.Either[languageplugin.BuildResult, error]) (*schema.Module, error) {
logger := log.FromContext(ctx)
config := c.Abs()

var result BuildResult
var result languageplugin.BuildResult
switch eitherResult := eitherResult.(type) {
case either.Right[BuildResult, error]:
case either.Right[languageplugin.BuildResult, error]:
return nil, fmt.Errorf("failed to build module: %w", eitherResult.Get())
case either.Left[BuildResult, error]:
case either.Left[languageplugin.BuildResult, error]:
result = eitherResult.Get()
}

Expand Down
31 changes: 0 additions & 31 deletions internal/buildengine/deps.go

This file was deleted.

28 changes: 0 additions & 28 deletions internal/buildengine/deps_test.go

This file was deleted.

Loading