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

Runtime: Set instance config with variables #4521

Merged
merged 9 commits into from
Apr 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 14 additions & 55 deletions admin/deployments.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,10 +83,6 @@ func (s *Service) createDeployment(ctx context.Context, opts *createDeploymentOp

// Prepare instance config
var connectors []*runtimev1.Connector
modelDefaultMaterialize, err := defaultModelMaterialize(opts.ProdVariables)
if err != nil {
return nil, err
}

// Always configure a DuckDB connector, even if it's not set as the default OLAP connector
connectors = append(connectors, &runtimev1.Connector{
Expand Down Expand Up @@ -180,18 +176,16 @@ func (s *Service) createDeployment(ctx context.Context, opts *createDeploymentOp

// Create the instance
_, err = rt.CreateInstance(ctx, &runtimev1.CreateInstanceRequest{
InstanceId: instanceID,
Environment: "prod",
OlapConnector: olapConnector,
RepoConnector: "admin",
AdminConnector: "admin",
AiConnector: "admin",
Connectors: connectors,
Variables: opts.ProdVariables,
Annotations: opts.Annotations.toMap(),
EmbedCatalog: false,
StageChanges: true,
ModelDefaultMaterialize: modelDefaultMaterialize,
InstanceId: instanceID,
Environment: "prod",
OlapConnector: olapConnector,
RepoConnector: "admin",
AdminConnector: "admin",
AiConnector: "admin",
Connectors: connectors,
Variables: opts.ProdVariables,
Annotations: opts.Annotations.toMap(),
EmbedCatalog: false,
})
if err != nil {
err2 := p.Deprovision(ctx, provisionID)
Expand Down Expand Up @@ -222,15 +216,6 @@ func (s *Service) UpdateDeployment(ctx context.Context, depl *database.Deploymen
return fmt.Errorf("cannot update deployment without specifying a valid branch")
}

var modelDefaultMaterialize *bool
if opts.Variables != nil { // if variables are nil, it means they were not changed
val, err := defaultModelMaterialize(opts.Variables)
if err != nil {
return err
}
modelDefaultMaterialize = &val
}

// Update the provisioned runtime if the version has changed
if opts.Version != depl.RuntimeVersion {
// Get provisioner from the set
Expand Down Expand Up @@ -293,11 +278,10 @@ func (s *Service) UpdateDeployment(ctx context.Context, depl *database.Deploymen
}

_, err = rt.EditInstance(ctx, &runtimev1.EditInstanceRequest{
InstanceId: depl.RuntimeInstanceID,
Connectors: connectors,
Annotations: opts.Annotations.toMap(),
Variables: opts.Variables,
ModelDefaultMaterialize: modelDefaultMaterialize,
InstanceId: depl.RuntimeInstanceID,
Connectors: connectors,
Annotations: opts.Annotations.toMap(),
Variables: opts.Variables,
})
if err != nil {
return err
Expand Down Expand Up @@ -458,28 +442,3 @@ func (da *DeploymentAnnotations) toMap() map[string]string {
res["project_name"] = da.projName
return res
}

// defaultModelMaterialize determines whether to materialize models by default for deployed projects.
// It defaults to true, but can be overridden with the __materialize_default variable.
func defaultModelMaterialize(vars map[string]string) (bool, error) {
// Temporary hack to enable configuring ModelDefaultMaterialize using a variable.
// Remove when we have a way to conditionally configure it using code files.

systemDefault := false

if vars == nil {
return systemDefault, nil
}

s, ok := vars["__materialize_default"]
if !ok {
return systemDefault, nil
}

val, err := strconv.ParseBool(s)
if err != nil {
return false, fmt.Errorf("invalid __materialize_default value %q: %w", s, err)
}

return val, nil
}
18 changes: 8 additions & 10 deletions cli/cmd/runtime/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,6 @@ type Config struct {
EmailSenderEmail string `split_words:"true"`
EmailSenderName string `split_words:"true"`
EmailBCC string `split_words:"true"`
DownloadRowLimit int64 `default:"10000" split_words:"true"`
ConnectionCacheSize int `default:"100" split_words:"true"`
QueryCacheSizeBytes int64 `default:"104857600" split_words:"true"` // 100MB by default
SecurityEngineCacheSize int `default:"1000" split_words:"true"`
Expand Down Expand Up @@ -232,15 +231,14 @@ func StartCmd(ch *cmdutil.Helper) *cobra.Command {

// Init server
srvOpts := &server.Options{
HTTPPort: conf.HTTPPort,
GRPCPort: conf.GRPCPort,
AllowedOrigins: conf.AllowedOrigins,
ServePrometheus: conf.MetricsExporter == observability.PrometheusExporter,
SessionKeyPairs: keyPairs,
AuthEnable: conf.AuthEnable,
AuthIssuerURL: conf.AuthIssuerURL,
AuthAudienceURL: conf.AuthAudienceURL,
DownloadRowLimit: &conf.DownloadRowLimit,
HTTPPort: conf.HTTPPort,
GRPCPort: conf.GRPCPort,
AllowedOrigins: conf.AllowedOrigins,
ServePrometheus: conf.MetricsExporter == observability.PrometheusExporter,
SessionKeyPairs: keyPairs,
AuthEnable: conf.AuthEnable,
AuthIssuerURL: conf.AuthIssuerURL,
AuthAudienceURL: conf.AuthAudienceURL,
}
s, err := server.NewServer(ctx, srvOpts, rt, logger, limiter, activityClient)
if err != nil {
Expand Down
13 changes: 11 additions & 2 deletions cli/pkg/local/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,15 @@ func NewApp(ctx context.Context, opts *AppOptions) (*App, error) {
return nil, err
}

// Merge opts.Variables with some local overrides of the defaults in runtime/drivers.InstanceConfig.
vars := map[string]string{
"rill.download_row_limit": "0", // 0 means unlimited
"rill.stage_changes": "false",
}
for k, v := range opts.Variables {
vars[k] = v
}

// Prepare connectors for the instance
var connectors []*runtimev1.Connector

Expand All @@ -186,7 +195,7 @@ func NewApp(ctx context.Context, opts *AppOptions) (*App, error) {
if opts.OlapDriver == DefaultOLAPDriver && olapDSN == DefaultOLAPDSN {
defaultOLAP = true
olapDSN = path.Join(dbDirPath, olapDSN)
val, err := isExternalStorageEnabled(dbDirPath, opts.Variables)
val, err := isExternalStorageEnabled(dbDirPath, vars)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -265,7 +274,7 @@ func NewApp(ctx context.Context, opts *AppOptions) (*App, error) {
AIConnector: aiConnector.Name,
CatalogConnector: catalogConnector.Name,
Connectors: connectors,
Variables: opts.Variables,
Variables: vars,
Annotations: map[string]string{},
WatchRepo: true,
// ModelMaterializeDelaySeconds: 30, // TODO: Enable when we support skipping it for the initial load
Expand Down
Loading
Loading