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

Don't use $HOME, hard-code it #66

Open
nsheff opened this issue Oct 13, 2020 · 3 comments
Open

Don't use $HOME, hard-code it #66

nsheff opened this issue Oct 13, 2020 · 3 comments

Comments

@nsheff
Copy link
Member

nsheff commented Oct 13, 2020

CWL rewrites $HOME for its runs, so it doesn't work with bulker shims, which by default are mounting $HOME as an env var...

I see no real value in keeping the variable in the bulker config, so when the config is initiated, by default we should probably simply resolve the environment variable at the config build time instead of maintaining the environment variable clear through to the containerized executable scripts.

@nsheff
Copy link
Member Author

nsheff commented Mar 2, 2021

The bulker config is a YacAttMap, which is a PathExAttMap -- so in theory it should be expanding those env vars... but it doesn't

Writing the config uses the .to_yaml() method on the YacAttMap, which is not populating $HOME.

import attmap
In [3]: attmap.PathExAttMap({"x": "$HOME"})
Out[3]: 
PathExAttMap
x: $HOME

In [4]: am = attmap.PathExAttMap({"x": "$HOME"})

In [7]: am.to_yaml()
Out[7]: 'x: $HOME\n'

In [8]: am.to_dict()
Out[8]: {'x': '$HOME'}

@stolarczyk what's the recommended way to make this populate env vars automatically since it doesn't do it by default for .to_yaml() -- it does it by default only for item or attribute access

In [5]: am.x
Out[5]: '/home/nsheff'
In [9]: am["x"]
Out[9]: '/home/nsheff'

Do we need a to_yaml(expand=True) option?

@stolarczyk
Copy link
Member

The bulker config is a YacAttMap, which is a PathExAttMap -- so in theory it should be expanding those env vars... but it doesn't

isn't the default use case to keep the env vars not expanded, to maintain the configs portability?

it doesn't do it by default for .to_yaml() -- it does it by default only for item or attribute access

don't want to speak for @vreuter, but to my understanding, that was exactly the idea behind this class

what's the recommended way to make this populate env vars automatically

I don't think there is a dedicated method that does that at the moment. Working with what we have now, I'd do this:

with open(path, 'w') as y:
    yaml.dump(x.to_dict(expand=True), y, default_flow_style=False)

Do we need a to_yaml(expand=True) option?

that would be useful

@nsheff
Copy link
Member Author

nsheff commented Mar 25, 2021

This all makes sense, and you're right, I think it's the way it should be. So, what I'm looking for is to_yaml(expand=True). Raised an issue there.

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

No branches or pull requests

2 participants