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

Remove dependency on global state in generated commands #595

Merged
merged 11 commits into from
Jul 25, 2023
Merged

Conversation

pietern
Copy link
Contributor

@pietern pietern commented Jul 21, 2023

Changes

Generated commands relied on global variables for flags and request payloads. This is difficult to test if a sequence of tests tries to run the same command with various arguments because the global state causes test interference. Moreover, it is impossible to run tests in parallel.

This change modifies the approach and turns every command group and command itself into a function that returns a *cobra.Command. All flags and request payloads are variables scoped to the command's initialization function. This means it is possible to construct independent copies of the CLI structure and fixes the test isolation issue.

The scope of this change is only the generated commands. The other commands will be changed accordingly in subsequent changes.

Tests

Unit and integration tests pass.

@pietern pietern marked this pull request as ready for review July 24, 2023 12:52
Copy link
Contributor

@mgyucht mgyucht left a comment

Choose a reason for hiding this comment

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

Couple small questions, but otherwise LGTM. Note that I only did a light pass on a couple of the generated files.

.codegen/cmds-account.go.tmpl Show resolved Hide resolved
.codegen/service.go.tmpl Show resolved Hide resolved
.codegen/service.go.tmpl Outdated Show resolved Hide resolved
.codegen/service.go.tmpl Show resolved Hide resolved
.codegen/service.go.tmpl Show resolved Hide resolved
@pietern pietern merged commit 3fa400f into main Jul 25, 2023
4 checks passed
@pietern pietern deleted the no-global-state branch July 25, 2023 18:19
pietern added a commit that referenced this pull request Jul 26, 2023
This change is another step towards a CLI without globals. Also see #595.
pietern added a commit that referenced this pull request Jul 26, 2023
## Changes

This change is another step towards a CLI without globals. Also see #595.

The flags for the root command are now encapsulated in struct types.

## Tests

Unit tests pass.
andrewnester pushed a commit that referenced this pull request Jul 26, 2023
## Changes

This change is another step towards a CLI without globals. Also see #595.

The flags for the root command are now encapsulated in struct types.

## Tests

Unit tests pass.
github-merge-queue bot pushed a commit that referenced this pull request Jul 27, 2023
## Changes

This removes the remaining dependency on global state and unblocks work
to parallelize integration tests. As is, we can already uncomment an
integration test that had to be skipped because of other tests tainting
global state. This is no longer an issue.

Also see #595 and #606.

## Tests

* Unit and integration tests pass.
* Manually confirmed the help output is the same.
@mgyucht mgyucht mentioned this pull request Jul 27, 2023
github-merge-queue bot pushed a commit that referenced this pull request Jul 27, 2023
Breaking Change:
* Require include glob patterns to be explicitly defined
([#602](#602)).

Bundles:
* Add support for more SDK config options
([#587](#587)).
* Add template renderer for Databricks templates
([#589](#589)).
* Fix formatting in renderer.go
([#593](#593)).
* Fixed python wheel test
([#608](#608)).
* Auto detect Python wheel packages and infer build command
([#603](#603)).
* Added support for artifacts building for bundles
([#583](#583)).
* Add support for cloning repositories
([#544](#544)).
* Add regexp compile helper function for templates
([#601](#601)).
* Add unit test that raw strings are printed as is
([#599](#599)).

Internal:
* Fix tests under ./cmd/configure if DATABRICKS_TOKEN is set
([#605](#605)).
* Remove dependency on global state in generated commands
([#595](#595)).
* Remove dependency on global state for the root command
([#606](#606)).
* Add merge_group trigger for build
([#612](#612)).
* Added support for build command chaining and error on missing wheel
([#607](#607)).
* Add TestAcc prefix to filer test and fix any failing tests
([#611](#611)).
* Add url parse helper function for templates
([#600](#600)).
* Remove dependency on global state for remaining commands
([#613](#613)).
* Update CHANGELOG template
([#588](#588)).
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.

3 participants