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

[Feature]: Custom ENV variable definitions #384

Closed
alejandrosame opened this issue Dec 2, 2022 · 2 comments
Closed

[Feature]: Custom ENV variable definitions #384

alejandrosame opened this issue Dec 2, 2022 · 2 comments
Assignees
Labels
enhancement New feature or request

Comments

@alejandrosame
Copy link

alejandrosame commented Dec 2, 2022

What's missing?

I found myself needing to define ENV variables when creating a custom kernel. The current function signature (see below) does not allow to pass ENV definitions to the available kernel.

https://github.com/tweag/jupyterWith/blob/8cb4db9bec9586ceb1269d33507fdaa31d8e358a/kernels/available/python/default.nix#L1-L22

Possible solution

The Rust available kernel already defines more than a PATH environment variable:

https://github.com/tweag/jupyterWith/blob/8cb4db9bec9586ceb1269d33507fdaa31d8e358a/kernels/available/rust/default.nix#L39-L40

I suggest that a new attribute extraEnvVars be introduced:

{ ... 
, extraEnvVars = [
   { VAR1 = VALUE1; }
   { VAR2 = VALUE2; }
  ]
, ...
} : 
{ ... }

The reason to format it as as a list of sets is:

  • to preserve the order in which they are defined. The builtins primitives attrNames and attrValues reorder names alphabetically.
  • so later jupyterWith could easily check for "protected" variables (e.g. PATH and RUST_SRC_PATH in the previous code snippet) that would need special handling (e.g. either drop or combine ).

Alternatives

No response

Additional context

No response

@alejandrosame alejandrosame added the enhancement New feature or request label Dec 2, 2022
@djacu
Copy link
Contributor

djacu commented Jan 24, 2023

Hey @alejandrosame

Thanks for submitting this. It is a really good idea!
There is a big PR @garbas and I have been working on for the last two months and it is going to change a lot of things.
I'll take a look at implementing this after the PR is merged.

I appreciate the thoughtfulness you put into the solution. I had thought about implementing a function to do something like this previously because some of the kernels require multiple environment variables being set. Like you mentioned above.

What about an interface like this?

extraEnvVars = {
  arg0 = [
   { VAR1 = VALUE1; }
   { VAR2 = VALUE2; }
  ];
  arg1 = [
   { VAR3 = VALUE3; }
   { VAR4 = VALUE4; }
  ];
};

Where arg0 and arg1 would be the different flags that you can use in makeWrapper.

So you could use it like

extraEnvVars = {
  set = [
    { PATH = /foo; }
    { RUST_PATH = /bar; }
  ];
  run = [
    ...
  ];
};

And then things under set would use the --set flag and things under run would use the --run flag. Maybe that is needlessly complicated, and all we need is set.

@GTrunSec
Copy link
Collaborator

closes by #524

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants