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

[BUG] - Configure conda-store allowed channels via nebari config #1594

Closed
Tracked by #1309
alimanfoo opened this issue Dec 8, 2022 · 6 comments · Fixed by #1701
Closed
Tracked by #1309

[BUG] - Configure conda-store allowed channels via nebari config #1594

alimanfoo opened this issue Dec 8, 2022 · 6 comments · Fixed by #1701
Labels
area: integration/conda-store needs: PR 📬 This item has been scoped and needs to be worked on type: bug 🐛 Something isn't working

Comments

@alimanfoo
Copy link
Contributor

alimanfoo commented Dec 8, 2022

Describe the bug

I'd like to configure the CondaStore.conda_allowed_channels via the nebari configuration file. This should be possible as described here:

conda_store:
  extra-settings:
     CondaStore:
       conda_allowed_channels = []

However, this does not validate according the nebari scheme, which requires "extra_settings".

Trying this configuration instead:

conda_store:
  extra_settings:
     CondaStore:
       conda_allowed_channels = []

...does validate but may result in a broken conda-store service.

Expected behavior

In general any configuration parameter should be possible to pass through to conda-store via nebari-config.yaml.

OS and architecture in which you are running Nebari

GKE

How to Reproduce the problem?

Try the example configs given above and deploy.

Command output

No response

Versions and dependencies used.

Nebari version 2022.11.1.

Compute environment

GCP

Integrations

conda-store

Anything else?

Originally discussed here: #1567.

@alimanfoo alimanfoo added needs: triage 🚦 Someone needs to have a look at this issue and triage type: bug 🐛 Something isn't working labels Dec 8, 2022
@iameskild
Copy link
Member

Thanks @alimanfoo for opening this! I haven't had a chance to test this yet. I will report back shortly :)

@pavithraes
Copy link
Member

@iameskild Thank you for taking a look, pinging just to surface this for you. :)

@costrouc
Copy link
Member

@alimanfoo I know this is a while back but would it be possible for you to share the Kubernetes logs for conda-store-server and conda-store-worker? I agree that this should not be crashing with this setting.

@viniciusdc
Copy link
Contributor

Hi @alimanfoo. Thanks for reporting this. If I understood correctly, you got a problem with the extra-config not being a standard key for conda_store, right? Have you encountered any issues with using extra_config instead, if so, could you post them as @costrouc commented above?

Just give some context, the reason why extra-config failed is mostly probably dues to a typo here:

"conda-store-extra-settings": config.get("conda_store", {}).get(
"extra_settings", {}
),
"conda-store-extra-config": config.get("conda_store", {}).get(
"extra_config", ""

and here:

nebari/nebari/schema.py

Lines 125 to 126 in 49bca71

extra_settings: typing.Optional[typing.Dict[str, typing.Any]] = {}
extra_config: typing.Optional[str] = ""

It should be extra-setting and extra-config to comply with conda-store docs.

This also means that for Nebari (at least the current versions), the standard key will be extra_config/extra_settings. They should not break conda-store because they are correctly passed later to conda-store with the correct syntax here:

extra-settings = var.conda-store-extra-settings
extra-config = var.conda-store-extra-config

I do agree that having those different structures to the same keys are confusing and unnecessary, so an action item would be to rename that to the correct syntax as per conda-store docs

@alimanfoo
Copy link
Contributor Author

Hi folks, thanks so much for following this one up, much appreciated. Just wanted to say apologies I'm unlikely to have time to do more on this in the short term and I'm afraid logs from when I tried this before will be long gone having been through a number of cycles of destroying and redeploying clusters since.

@pavithraes pavithraes added needs: PR 📬 This item has been scoped and needs to be worked on and removed needs: triage 🚦 Someone needs to have a look at this issue and triage labels Dec 20, 2022
@dharhas
Copy link
Member

dharhas commented Feb 5, 2023

@viniciusdc @iameskild @costrouc I need to do the same thing and it is pretty hard to parse from the conversation above what actually needs to be done to allow other channels. We urgently need this on demo.nebari.dev

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: integration/conda-store needs: PR 📬 This item has been scoped and needs to be worked on type: bug 🐛 Something isn't working
Projects
Development

Successfully merging a pull request may close this issue.

6 participants