Skip to content

fix: clarify use of cwms-cli env activate - #269

Merged
msweier merged 9 commits into
mainfrom
fix/env_man
Sep 24, 2026
Merged

msweier merged 9 commits into
mainfrom
fix/env_man

Conversation

@msweier

@msweier msweier commented Sep 22, 2026 •

Copy link
Copy Markdown
Collaborator

Clarifies how cwms-cli env activate works. The spawning of a new shell with environment variables like CDA_API_ROOT and CDA_API_KEY may be overriden if these are specified in a .bash_profile or .bashrc file. This provides better documentation on how to deal with that.

  1. Provides instructions on how to use eval "$(cwms-cli env export <name> --format bash)" in the bashrc to have cwms-cli provide the environment variables on an onprem server.
  2. Clarifies how cwms-cli env activate <name> may not work as intended if environment variables (e.g. CDA_API_ROOT) are defined in the bashrc and provided documentation on how to avoid that.

AI Assisted

@msweier
msweier requested a review from krowvin as a code owner September 22, 2026 12:51
@msweier msweier assigned krowvin and unassigned krowvin Sep 22, 2026
@msweier
msweier marked this pull request as draft September 22, 2026 13:32
@msweier
msweier marked this pull request as ready for review September 23, 2026 11:37
@msweier
msweier requested a review from Enovotny September 23, 2026 11:42
@msweier

msweier commented Sep 23, 2026 •

Copy link
Copy Markdown
Collaborator Author

@Enovotny may want to consider revising the Setting up CDA writes and cwms-python on servers section on confluence to include using cwms-cli for storing the CDA_API_KEY. That way we don't have multiple places to store it. The help docs have the details.

@krowvin krowvin left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This PR looks to focus around documentation of the issue we saw where a user tried to run cwms-cli env activate on their Unix box.

It failed, and they came to us saying it does not work.

If we only include documentation, I think, this will happen again and we will have to then send them the new doc links.

To take us out of the middle,
I propose we either:

  1. Wrap it in a try/except block for the specific error
  2. Add an if statement and actively get ahead of it

To then say "You must manually run the activate command, shell spawning is not supported on your system:"

Then you print

eval "$(cwms-cli env export <name> --format bash)"

and you print

It's not as good as running it for them, but it's a nice holdover to say we are aware and here's how they can activate.

You could also have a note to the docs link "To make this change permanent read the docs here:"

If you do go this route, see how I handle docs links in other places of CWMS-CLI

@msweier

msweier commented Sep 24, 2026

Copy link
Copy Markdown
Collaborator Author

Ok I made a couple more improvements here:

  1. A warning shows up with cwms-cli env activate <name> tailored for what platform you are running to say a bashrc or windows profile could overwrite the variables in the spawned shell and you better check to make sure.
image 2. I updated `cwms-cli env show` to compare the current environment variables with those stored by CDA and flag a warning if they don't match. image

@krowvin
krowvin self-requested a review September 24, 2026 13:42
Comment thread cwmscli/commands/env.py

@krowvin krowvin left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks good! Color is a nice touch!

@msweier
msweier merged commit 0c4635a into main Sep 24, 2026
12 checks passed
@msweier
msweier deleted the fix/env_man branch September 24, 2026 14:04
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