-
Notifications
You must be signed in to change notification settings - Fork 168
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
Fix channels_remap
handling in extra_envs
configuration
#808
Conversation
constructor/main.py
Outdated
if config_key == "environment_file": | ||
env_config[config_key] = abspath(join(dir_path, value)) | ||
elif config_key == "channels_remap": | ||
env_config[config_key] = [ | ||
{"src": item["src"].strip(), "dest": "dest".strip()} for item in value |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
{"src": item["src"].strip(), "dest": "dest".strip()} for item in value | |
{"src": item["src"].strip(), "dest": item["dest"].strip()} for item in value |
The way this code is written, it looks like the dest
will always be "dest". I'm surprised the tests pass - sounds like the example is maybe not as representative as they should be?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops, good catch. This would only be caught if we try to update the resulting installation or run conda list
on them because that's where the remapped channels would show up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extended a test
Description
An oversight in the strip logic does not handle
channel_remaps
correctly (it assumed list of str, but we have list of dict[str, str] there).This input file:
results in the following error:
Checklist - did you ...
news
directory (using the template) for the next release's release notes?