Fixes `denylist_channels` channel bug 🐛 #14405

travishathaway · 2024-11-22T14:54:41Z

Description

Checklist - did you ...

Add a file to the news directory (using the template) for the next release's release notes?
Add / update necessary tests?
~~Add / update outdated documentation?~~

travishathaway · 2024-11-22T14:55:59Z

Still needs tests.

travishathaway · 2024-11-22T15:03:29Z

I'm still not sure I like the error message because it's not as informative as it could be. It says what's wrong but does not suggest what the user could do differently. I'd like to add some more information here.

I think the message should read something more like:

You have encountered this error because you are currently attempting to use a 
channel defined in your "denylist_channels" setting. To see a complete list of these 
channels, run `conda config --show denylist_channels` and make sure you are not 
using these channels either by specifying them at the command line or in your 
`.condarc` file

That is a little long though 😅. I'm open to suggestions for make it more concise.

codspeed-hq · 2024-11-22T15:10:41Z

CodSpeed Performance Report

Merging #14405 will not alter performance

_{Comparing travishathaway:bugfix-14403 (75e0627) with main (dc1013e)}

Summary

✅ 21 untouched benchmarks

conda/base/context.py

…h must test multiple commands under similar circumstances

tests/cli/test_all_commands.py

conda/base/context.py

conda/cli/helpers.py

conda/base/context.py

conda/core/index.py

conda/base/context.py

conda/cli/helpers.py

conda/core/index.py

Co-authored-by: Jannis Leidel <[email protected]>

…ugfix-14403

travishathaway · 2024-11-27T14:01:37Z

@jezdez,

Renamed the function and now have the deprecation warnings in place.

conda/core/index.py

Co-authored-by: Ken Odegard <[email protected]>

* Rename check_channel_allowlist to validate_channels to better reflect its purpose of validating both allowlist and denylist configurations * Update function to return the validated channels tuple * Update all references in code to use the new name * Update deprecation notice in check_allowlist to reference new name * Fix news entry formatting for consistency Fixes conda#14405

kenodegard

Looks great! Minor suggestions but otherwise this is ready to merge!

conda/cli/helpers.py

conda/base/context.py

Co-authored-by: Ken Odegard <[email protected]>

jezdez · 2024-12-02T16:44:30Z

@travishathaway this also needs removing the IndexSet calls throughout the code, after moving it inline

jezdez · 2024-12-02T21:26:48Z

Huzzah!

* initial attempt at fixing denylist_channels bug; still needs tests * adding a news file * making sure the validation is applied for each return statement * fixing broken test * adding a new testing module to handle the tests and future tests which must test multiple commands under similar circumstances * suggestions for changes from pull request review * suggestions from pull request review * Apply suggestions from code review Co-authored-by: Jannis Leidel <[email protected]> * more suggestions from pull request review * Update conda/core/index.py Co-authored-by: Ken Odegard <[email protected]> * Move tuple into check_channel_allowlist. * Rename check_channel_allowlist to validate_channels * Rename check_channel_allowlist to validate_channels to better reflect its purpose of validating both allowlist and denylist configurations * Update function to return the validated channels tuple * Update all references in code to use the new name * Update deprecation notice in check_allowlist to reference new name * Fix news entry formatting for consistency Fixes #14405 * Fix lint. * Fix tests. * Minor fix. * The argparse args are an AttrDict and need converting first. --------- Co-authored-by: Jannis Leidel <[email protected]> Co-authored-by: Ken Odegard <[email protected]>

* initial attempt at fixing denylist_channels bug; still needs tests * adding a news file * making sure the validation is applied for each return statement * fixing broken test * adding a new testing module to handle the tests and future tests which must test multiple commands under similar circumstances * suggestions for changes from pull request review * suggestions from pull request review * Apply suggestions from code review Co-authored-by: Jannis Leidel <[email protected]> * more suggestions from pull request review * Update conda/core/index.py Co-authored-by: Ken Odegard <[email protected]> * Move tuple into check_channel_allowlist. * Rename check_channel_allowlist to validate_channels * Rename check_channel_allowlist to validate_channels to better reflect its purpose of validating both allowlist and denylist configurations * Update function to return the validated channels tuple * Update all references in code to use the new name * Update deprecation notice in check_allowlist to reference new name * Fix news entry formatting for consistency Fixes conda#14405 * Fix lint. * Fix tests. * Minor fix. * The argparse args are an AttrDict and need converting first. --------- Co-authored-by: Jannis Leidel <[email protected]> Co-authored-by: Ken Odegard <[email protected]>

* Fixes `denylist_channels` channel bug 🐛 (#14405) * initial attempt at fixing denylist_channels bug; still needs tests * adding a news file * making sure the validation is applied for each return statement * fixing broken test * adding a new testing module to handle the tests and future tests which must test multiple commands under similar circumstances * suggestions for changes from pull request review * suggestions from pull request review * Apply suggestions from code review Co-authored-by: Jannis Leidel <[email protected]> * more suggestions from pull request review * Update conda/core/index.py Co-authored-by: Ken Odegard <[email protected]> * Move tuple into check_channel_allowlist. * Rename check_channel_allowlist to validate_channels * Rename check_channel_allowlist to validate_channels to better reflect its purpose of validating both allowlist and denylist configurations * Update function to return the validated channels tuple * Update all references in code to use the new name * Update deprecation notice in check_allowlist to reference new name * Fix news entry formatting for consistency Fixes #14405 * Fix lint. * Fix tests. * Minor fix. * The argparse args are an AttrDict and need converting first. --------- Co-authored-by: Jannis Leidel <[email protected]> Co-authored-by: Ken Odegard <[email protected]> * changing dependencies in integration test --------- Co-authored-by: Travis Hathaway <[email protected]> Co-authored-by: Ken Odegard <[email protected]>

travishathaway added 2 commits November 22, 2024 15:53

initial attempt at fixing denylist_channels bug; still needs tests

bb9cefa

adding a news file

f967234

conda-bot added the cla-signed [bot] added once the contributor has signed the CLA label Nov 22, 2024

kenodegard reviewed Nov 22, 2024

View reviewed changes

conda/base/context.py Outdated Show resolved Hide resolved

travishathaway added 3 commits November 22, 2024 17:09

making sure the validation is applied for each return statement

9cf86e9

fixing broken test

5c03b00

adding a new testing module to handle the tests and future tests whic…

f4823aa

…h must test multiple commands under similar circumstances

travishathaway marked this pull request as ready for review November 25, 2024 12:49

travishathaway requested a review from a team as a code owner November 25, 2024 12:49

kenodegard requested changes Nov 25, 2024

View reviewed changes

tests/cli/test_all_commands.py Outdated Show resolved Hide resolved

conda/base/context.py Outdated Show resolved Hide resolved

conda/base/context.py Outdated Show resolved Hide resolved

travishathaway added 2 commits November 26, 2024 12:24

suggestions for changes from pull request review

24d4e11

Merge branch 'main' into bugfix-14403

5d44c04

travishathaway requested a review from kenodegard November 26, 2024 11:25

jezdez requested changes Nov 26, 2024

View reviewed changes

conda/cli/helpers.py Outdated Show resolved Hide resolved

conda/cli/helpers.py Outdated Show resolved Hide resolved

conda/cli/helpers.py Outdated Show resolved Hide resolved

conda/base/context.py Outdated Show resolved Hide resolved

conda/core/index.py Show resolved Hide resolved

travishathaway added 2 commits November 27, 2024 09:35

Merge branch 'main' into bugfix-14403

6c88c8e

suggestions from pull request review

fc471d9

travishathaway requested a review from jezdez November 27, 2024 08:36

jezdez reviewed Nov 27, 2024

View reviewed changes

conda/base/context.py Outdated Show resolved Hide resolved

conda/cli/helpers.py Outdated Show resolved Hide resolved

conda/core/index.py Show resolved Hide resolved

travishathaway and others added 3 commits November 27, 2024 14:17

Apply suggestions from code review

2af96ff

Co-authored-by: Jannis Leidel <[email protected]>

more suggestions from pull request review

f4fa8ae

Merge branch 'bugfix-14403' of github.com:travishathaway/conda into b…

cb4cffc

…ugfix-14403

travishathaway requested a review from jezdez November 27, 2024 14:01

kenodegard reviewed Nov 27, 2024

View reviewed changes

conda/core/index.py Outdated Show resolved Hide resolved

jezdez and others added 2 commits November 28, 2024 08:10

Update conda/core/index.py

7168724

Co-authored-by: Ken Odegard <[email protected]>

Move tuple into check_channel_allowlist.

1d18a93

jezdez force-pushed the bugfix-14403 branch from 3e3c94b to b470f3c Compare November 28, 2024 14:36

jezdez added 4 commits November 28, 2024 15:37

Fix lint.

ce7b703

Fix tests.

220232c

Minor fix.

913ea67

The argparse args are an AttrDict and need converting first.

809c386

jezdez previously approved these changes Nov 29, 2024

View reviewed changes

jezdez requested a review from kenodegard November 29, 2024 11:18

Merge branch 'main' into bugfix-14403

7982572

travishathaway enabled auto-merge (squash) December 2, 2024 11:20

kenodegard requested changes Dec 2, 2024

View reviewed changes

conda/cli/helpers.py Outdated Show resolved Hide resolved

conda/base/context.py Outdated Show resolved Hide resolved

conda/base/context.py Outdated Show resolved Hide resolved

Apply suggestions from code review

6b1e9bb

Co-authored-by: Ken Odegard <[email protected]>

travishathaway dismissed jezdez’s stale review via 6b1e9bb December 2, 2024 16:41

travishathaway requested a review from kenodegard December 2, 2024 16:41

travishathaway and others added 6 commits December 2, 2024 17:45

fixing class name

4b083b0

fixing pre-commit error

b71ae84

Remove redundant IndexedSet usage in context.channels

fe1bfca

Remove redundant base_urls expansion and collapse list comprehension

498d928

Use relative import

fe7f86f

Simplify

75e0627

kenodegard approved these changes Dec 2, 2024

View reviewed changes

travishathaway merged commit 290bc18 into conda:main Dec 2, 2024
62 checks passed

jezdez mentioned this pull request Dec 9, 2024

24.11.x Fixes denylist_channels channel bug 🐛 #14431

Merged

3 tasks

jezdez mentioned this pull request Dec 10, 2024

Release conda 24.11.1. AnacondaRecipes/conda-feedstock#60

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes `denylist_channels` channel bug 🐛 #14405

Fixes `denylist_channels` channel bug 🐛 #14405

travishathaway commented Nov 22, 2024 •

edited

Loading

travishathaway commented Nov 22, 2024

travishathaway commented Nov 22, 2024

codspeed-hq bot commented Nov 22, 2024 •

edited

Loading

travishathaway commented Nov 27, 2024

kenodegard left a comment

jezdez commented Dec 2, 2024

jezdez commented Dec 2, 2024

Fixes denylist_channels channel bug 🐛 #14405

Fixes denylist_channels channel bug 🐛 #14405

Conversation

travishathaway commented Nov 22, 2024 • edited Loading

Description

Checklist - did you ...

travishathaway commented Nov 22, 2024

travishathaway commented Nov 22, 2024

codspeed-hq bot commented Nov 22, 2024 • edited Loading

CodSpeed Performance Report

Merging #14405 will not alter performance

Summary

travishathaway commented Nov 27, 2024

kenodegard left a comment

Choose a reason for hiding this comment

jezdez commented Dec 2, 2024

jezdez commented Dec 2, 2024

Fixes `denylist_channels` channel bug 🐛 #14405

Fixes `denylist_channels` channel bug 🐛 #14405

travishathaway commented Nov 22, 2024 •

edited

Loading

codspeed-hq bot commented Nov 22, 2024 •

edited

Loading