Home Original page

feat(organization): add --workspace-id flag to org switch for non-interactive use by ronidelacruz-astro · Pull Request #1938 · astronomer/astro-cli

@ronidelacruz-astro

Description

Add --workspace-id (-w) to astro organization switch to allow non-interactive workspace switching after changing org.

🎟 Issue(s)

Related #1805

🧪 Functional Testing

  • astro organization switch → still prompts if multiple workspaces.
  • astro organization switch --workspace-id → switches org and workspace without prompt.
  • Verified via unit tests (TestOrganizationSwitch).

📸 Screenshots

Add screenshots to illustrate the validity of these changes.

📋 Checklist

  • Rebased from the main (or release if patching) branch (before testing)
  • Ran make test before taking out of draft
  • Ran make lint before taking out of draft
  • Added/updated applicable tests
  • Tested against Astro-API (if necessary).
  • Tested against Houston-API and Astronomer (if necessary).
  • Communicated to/tagged owners of respective clients potentially impacted by these changes.
  • Updated any related documentation

@coveralls-official

@ronidelacruz-astro

@ronidelacruz-astro

ianbuss

Choose a reason for hiding this comment

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

Couple of nits but otherwise looks good!

_, err := execOrganizationCmd(cmdArgs...)
assert.NoError(t, err)
origOrgSwitch := orgSwitch
orgSwitch = func(orgName string, coreClient astrocore.CoreClient, platformCoreClient astroplatformcore.CoreClient, out io.Writer, shouldDisplayLoginLink bool) error {

Choose a reason for hiding this comment

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

Looks like this is an existing pattern in this code base but this is unlikely to be "safe" for parallel test runs if we were to ever use those. Might be worth just adding a comment to that effect. You've done the right thing here @ronidelacruz-astro by adding the defer function to put the function back at the end of this test, which looks like it's missing from other tests.

feluelle

Choose a reason for hiding this comment

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

LGTM, I would also have commented the same nit as Ian about the inline err check.

@ronidelacruz-astro @ianbuss 

Co-authored-by: Ian Buss <ianbuss@users.noreply.github.com>