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

Add conda config warning docs/message; add duration log for actions #823

Merged

Conversation

peytondmurray
Copy link
Contributor

@peytondmurray peytondmurray commented May 13, 2024

Fixes #810.

Description

This PR adds:

  1. A warning message about environment variables used by conda-store to override user conda configuration, which should help clear up user uncertainty about the output of conda config.
  2. Some information in the documentation about the above ☝️
  3. An info-level log message is now emitted displaying the amount of time each action takes.

Pull request checklist

  • Did you test this change locally? Yes, but not all tests pass locally on main. The ones I expected to pass locally have passed.
  • Did you update the documentation (if required)?
  • Did you add/update relevant tests for this change (if required)?

Notes

  • To see that this works locally, run any action. Alternatively, run pytest conda_store_server/test/test_actions.py::test_action_decorator - that test specifically checks for the run time information output to stdout that this PR implements.

Copy link

netlify bot commented May 13, 2024

Deploy Preview for conda-store ready!

Name Link
🔨 Latest commit 3f64175
🔍 Latest deploy log https://app.netlify.com/sites/conda-store/deploys/668596d44c6db40008ded192
😎 Deploy Preview https://deploy-preview-823--conda-store.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@peytondmurray peytondmurray force-pushed the make-logs-user-friendly-810 branch from 6bc4145 to 45b8455 Compare May 13, 2024 06:09
@peytondmurray peytondmurray force-pushed the make-logs-user-friendly-810 branch from 45b8455 to b58bb6a Compare May 28, 2024 14:33
@peytondmurray peytondmurray force-pushed the make-logs-user-friendly-810 branch 2 times, most recently from 40e5c7a to ee1d14d Compare June 17, 2024 22:08
@peytondmurray peytondmurray marked this pull request as ready for review June 17, 2024 22:10
@pavithraes pavithraes self-requested a review June 18, 2024 16:04
@peytondmurray peytondmurray force-pushed the make-logs-user-friendly-810 branch from c58eb4a to 437cc96 Compare June 25, 2024 00:31
Copy link
Contributor

@gabalafou gabalafou left a comment

Choose a reason for hiding this comment

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

Did not test but the code looks good to me.

The log message may be ambiguous, not sure, but at any rate it does the job of letting the user know that something is up with conda config, which is the main point.

Comment on lines +31 to +34
"Note that the output of `conda config --show` displayed below only reflects "
"settings in the conda configuration file, which might be overridden by "
"variables required to be set by conda-store via the environment. Overridden "
f"settings: CONDA_FLAGS={conda_flags}"
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm getting three distinct messages from this paragraph:

  1. conda config shows settings in Conda configuration file
  2. Some of the settings shown may therefore not be accurate because they have been overridden by Conda Store.
  3. Here are some (all?) of the settings that have been overridden in Conda Store.

Is that accurate?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, these were the three things I was trying to convey. As far as I know these are all of the overridden settings as well.

that are actually used by conda-store. In particular, `conda-store` internally
sets `CONDA_FLAGS=--strict-channel-priority`, overriding the channel priority in
the conda configuration file. Please keep this in mind when using `conda config`
to inspect your conda configuration.
Copy link
Contributor

@kcpevey kcpevey Jul 3, 2024

Choose a reason for hiding this comment

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

This is also (and perhaps more frequently) an issue when you view the logs for conda-store since the config is printed there as well. I think you've covered it in those logs already though.

Copy link
Contributor

@kcpevey kcpevey left a comment

Choose a reason for hiding this comment

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

I just had a minor suggestion, otherwise, this looks great! Thanks @peytondmurray !

@peytondmurray peytondmurray merged commit 9f24bd6 into conda-incubator:main Jul 3, 2024
27 checks passed
@peytondmurray peytondmurray deleted the make-logs-user-friendly-810 branch July 3, 2024 19:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: documentation 📖 Improvements or additions to documentation area: user experience 👩🏻‍💻 Items impacting the end-user experience type: enhancement 💅🏼
Projects
Status: Done 💪🏾
Development

Successfully merging this pull request may close these issues.

[ENH] - Make logs more user-friendly
3 participants