Skip to content

Remove environment bridges and consolidate all inputs through typed configuration - #151

Open
Cianidos wants to merge 3 commits into
refactor/issue-143-plain-go-configfrom
fix/issue-138-env-bridges
Open

Remove environment bridges and consolidate all inputs through typed configuration#151
Cianidos wants to merge 3 commits into
refactor/issue-143-plain-go-configfrom
fix/issue-138-env-bridges

Conversation

@Cianidos

Copy link
Copy Markdown
Contributor

Closes #138.

Removes the historical process-environment bridges that bypassed the typed
configuration system, so every user input resolves once into typed
run/workload/driver configuration and flows through native APIs. Environment
variables remain input sources (e.g. LOG_LEVEL, workload-param names), never
internal transport between command, bench, workload, and driver layers.

Changes:

  • --steps/--no-steps are explicit bench.Run inputs; the bench step filter
    no longer reads STROPPY_STEPS/STROPPY_NO_STEPS.
  • execute_sql SQL source is now the typed --sql-body / --sql-file
    workload params. Inline SQL, .sql path, and the sql positional bind as CLI
    inputs; STROPPY_SQL_BODY stays accepted as a legacy process-env alias.
  • Legacy POOL_SIZE shorthand no longer post-applies after driver extras, so
    -D pool.maxConns / -D pool.minConns and the config-file pool block win
    deterministically.
  • CSV workload identity comes only from the ?workload= URL option; the
    STROPPY_CSV_WORKLOAD fallback is removed.
  • Dead STROPPY_DRIVER_N serialization/probe helpers and the generic
    pkg/bench env readers (Env/EnvInt/EnvFloat) are deleted.
  • k6Args, k6Config, and driver-level defaultTxIsolation are removed from
    the v6 config schema and rejected; the docs schema and help topics are
    regenerated/updated.

Verification: make build, go test ./..., go vet ./..., and scoped
golangci-lint v2.12.2 over the changed packages are all clean.

Resolve all user inputs once into typed run/workload/driver configuration
and pass them through native APIs. Environment variables stay input
sources but are no longer internal transport between command, bench,
workload, and driver layers:

- step filters (--steps/--no-steps) are explicit Run inputs, read by the
  bench step filter directly instead of STROPPY_STEPS/STROPPY_NO_STEPS
- execute_sql SQL source is the typed sql-body/sql-file workload params
  (inline SQL / .sql file / sql positional bind as CLI inputs)
- legacy POOL_SIZE shorthand no longer post-applies after -D extras, so
  explicit -D pool.maxConns / pool.minConns win deterministically
- CSV workload identity comes only from the ?workload= URL option, never
  from STROPPY_CSV_WORKLOAD
- dead STROPPY_DRIVER_N serialization and probe-env helpers removed, and
  pkg/bench generic environment readers (Env/EnvInt/EnvFloat) deleted
- k6Args, k6Config, and driver-level defaultTxIsolation removed from the
  v6 config schema and rejected with migration guidance

Refs #138
@coderabbitai

coderabbitai Bot commented Aug 20, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 2c0e9676-03e0-47cd-b3ef-0d5023e2aa21

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

- add k6Args/k6Config migration guidance at config load (typed
  executor/vus/iterations/duration), mirroring the -D defaultTxIsolation
  --tx-isolation error
- drop defaultTxIsolation from AGENTS.md driver-flag list
- cover the defaultTxIsolation rejection with a test

Refs #138
@Cianidos

Copy link
Copy Markdown
Contributor Author

Blocked by skipped parent and config-contract decision

This PR cannot be mechanically retargeted to current main after #150 was skipped.

A dry rebase of only #151's commits (1e982655..e4ad1b50) onto current main stopped on the first commit with 14 conflicts. Most importantly, #151 modifies pkg/config, which exists only in #150; current main still uses the frozen protobuf config and protojson. Accepting the delete/modify conflicts would silently reintroduce #150 rather than isolate #151.

Retargeting also requires a functional policy decision for k6Args, k6Config, and defaultTxIsolation:

  1. Keep them accepted but inert in the current protobuf contract.
  2. Retain the protobuf API/descriptors but reject them at JSON/CLI boundaries.
  3. Remove the fields, getters, descriptors, and schema entirely.

These options differ in compatibility, error timing, and public API. They are not mechanical conflict resolutions.

GitHub currently reports this PR clean only against refactor/issue-143-plain-go-config. Merging it as-is would update that blocked branch, not main; retargeting it directly to main would include all four #150 commits as well as this PR's three commits.

Please resolve #150's config policy, choose the legacy-field behavior above, then restack only the selected environment-bridge changes on current main. Per the sequential merge rule, #151 is skipped for now; no branch changes were pushed.

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.

1 participant