Skip to content

Comments

feat: inject BRUIN_VARS_SCHEMA for type-aware variable coercion#1718

Merged
karakanb merged 4 commits intomainfrom
feature/type-aware-var-coercion
Feb 22, 2026
Merged

feat: inject BRUIN_VARS_SCHEMA for type-aware variable coercion#1718
karakanb merged 4 commits intomainfrom
feature/type-aware-var-coercion

Conversation

@sabrikaragonen
Copy link
Contributor

@sabrikaragonen sabrikaragonen commented Feb 22, 2026

Summary

  • Adds SchemaMap() method to Variables that returns type definitions without the default key
  • Modifies envInjectVariables to marshal and set BRUIN_VARS_SCHEMA alongside BRUIN_VARS
  • Enables the Python SDK to coerce CLI string overrides (--var count=42) to proper types

Companion PR

  • Python side (reads schema and coerces): bruin-data/python-sdk — feature/type-aware-var-coercion

Test plan

  • 3 unit tests for SchemaMap() in variables_test.go
  • 2 unit tests for BRUIN_VARS_SCHEMA injection in env/variables_test.go
  • All existing tests still pass
  • E2E test after Python PR is merged

🤖 Generated with Claude Code

Add SchemaMap() to Variables that returns type definitions without
defaults. envInjectVariables now marshals and sets BRUIN_VARS_SCHEMA
so the Python SDK can coerce CLI string overrides to proper types.

Co-Authored-By: Claude Opus 4.6 <[email protected]>
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

4 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 22, 2026

Additional Comments (1)

pkg/env/variables.go
Asymmetric handling of empty variables

When variables is empty, BRUIN_VARS is explicitly set to "{}" but BRUIN_VARS_SCHEMA is omitted entirely. This means the Python SDK must handle two distinct cases: (1) BRUIN_VARS_SCHEMA is absent, and (2) BRUIN_VARS_SCHEMA is present with content. Consider setting BRUIN_VARS_SCHEMA to "{}" alongside BRUIN_VARS for consistency, so the companion Python SDK can always expect the env var to exist when BRUIN_VARS does.

	if len(variables) == 0 {
		env["BRUIN_VARS"] = "{}"
		env["BRUIN_VARS_SCHEMA"] = "{}"
		return env, nil
	}
Prompt To Fix With AI
This is a comment left during a code review.
Path: pkg/env/variables.go
Line: 64-67

Comment:
**Asymmetric handling of empty variables**

When `variables` is empty, `BRUIN_VARS` is explicitly set to `"{}"` but `BRUIN_VARS_SCHEMA` is omitted entirely. This means the Python SDK must handle two distinct cases: (1) `BRUIN_VARS_SCHEMA` is absent, and (2) `BRUIN_VARS_SCHEMA` is present with content. Consider setting `BRUIN_VARS_SCHEMA` to `"{}"` alongside `BRUIN_VARS` for consistency, so the companion Python SDK can always expect the env var to exist when `BRUIN_VARS` does.

```suggestion
	if len(variables) == 0 {
		env["BRUIN_VARS"] = "{}"
		env["BRUIN_VARS_SCHEMA"] = "{}"
		return env, nil
	}
```

How can I resolve this? If you propose a fix, please make it concise.

sabrikaragonen and others added 3 commits February 22, 2026 17:22
Ensures BRUIN_VARS_SCHEMA is always present alongside BRUIN_VARS
for consistency, so the Python SDK doesn't need to handle the
absent-env-var case separately.

Co-Authored-By: Claude Opus 4.6 <[email protected]>
@karakanb karakanb merged commit a9691a3 into main Feb 22, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants