Runtime: Set instance config with variables (#4521)

* Runtime: Set instance config with variables * Add key for download row limit * Config for pivot cell limit * Fix test and lint * Self review * Self review * Add more config keys * Review
rilldata · Apr 9, 2024 · 121c0b5 · 121c0b5 · github-actions · Apr 9, 2024
1 parent 7c789f8
commit 121c0b5
Show file tree

Hide file tree

Showing 21 changed files with 1,354 additions and 1,530 deletions.
diff --git a/admin/deployments.go b/admin/deployments.go
@@ -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{
@@ -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)
@@ -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
@@ -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
@@ -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
-}
diff --git a/cli/cmd/runtime/start.go b/cli/cmd/runtime/start.go
@@ -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"`
@@ -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 {

diff --git a/cli/pkg/local/app.go b/cli/pkg/local/app.go
@@ -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
 
@@ -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
 		}
@@ -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