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

refact: conda/mamba create/update env, fix user permissions #933

Merged
merged 20 commits into from
Oct 4, 2022

Conversation

kemingy
Copy link
Member

@kemingy kemingy commented Sep 22, 2022

Signed-off-by: Keming kemingyang@tensorchord.ai

  • split "create user envd" and "chown envd"
  • fix conda/mamba create new env from env YAML file

Signed-off-by: Keming <kemingyang@tensorchord.ai>
Signed-off-by: Keming <kemingyang@tensorchord.ai>
Signed-off-by: Keming <kemingyang@tensorchord.ai>
Signed-off-by: Keming <kemingyang@tensorchord.ai>
Signed-off-by: Keming <kemingyang@tensorchord.ai>
Signed-off-by: Keming <kemingyang@tensorchord.ai>
Signed-off-by: Keming <kemingyang@tensorchord.ai>
Signed-off-by: Keming <kemingyang@tensorchord.ai>
@muniu-bot
Copy link

muniu-bot bot commented Sep 22, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kemingy

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@muniu-bot muniu-bot bot added the approved label Sep 22, 2022
@kemingy kemingy changed the title Conda create env feat: conda/mamba create env from env.yaml file Sep 22, 2022
Signed-off-by: Keming <kemingyang@tensorchord.ai>
pkg/lang/ir/conda.go Outdated Show resolved Hide resolved
kemingy and others added 3 commits September 22, 2022 17:48
Co-authored-by: Ce Gao <ce.gao@outlook.com>
Signed-off-by: Keming <kemingy94@gmail.com>
Signed-off-by: Keming <kemingyang@tensorchord.ai>

Signed-off-by: Keming <kemingyang@tensorchord.ai>
Signed-off-by: Keming <kemingyang@tensorchord.ai>
Copy link
Member

@VoVAllen VoVAllen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you need also chown on pip package path? Also if there's pip package in conda yaml file, is it installed under root user or envd user?

@VoVAllen
Copy link
Member

Also will conda install anything under current user folder?

@kemingy
Copy link
Member Author

kemingy commented Sep 22, 2022

Do you need also chown on pip package path? Also if there's pip package in conda yaml file, is it installed under root user or envd user?

Pip package path should be under "/opt/conda/envs/envd".

Tested "install -e ." can work.

@kemingy
Copy link
Member Author

kemingy commented Sep 22, 2022

Also will conda install anything under current user folder?

Does "current user folder" mean "/home/envd"? This one has also been chowned.

@VoVAllen
Copy link
Member

I mean if the installation process is executed under root user, will it install anything to /root/XXXX?

@gaocegege
Copy link
Member

gaocegege commented Sep 22, 2022

Pip package path should be under "/opt/conda/envs/envd".

Could you please verify if pip cache works after the change? I am not sure if /home/envd/.pip/cache still work

@kemingy
Copy link
Member Author

kemingy commented Sep 22, 2022

I mean if the installation process is executed under root user, will it install anything to /root/XXXX?

Not sure if some libraries will modify the /root/ dir. Do you have any examples? I tried jupyter, it looks good.

Pip package path should be under "/opt/conda/envs/envd".

Could you please verify if pip cache works after the change? I am not sure if /home/envd/.pip/cache still work

The cache seems doesn't work at all. I tried to use /root/.cache/pip, /opt/conda/pkgs, but it doesn't work. Let me try to figure it out.

@VoVAllen
Copy link
Member

You can try install pip package such as numpy in conda. And check whether user can import numpy under user envd

@gaocegege
Copy link
Member

And, if we use root in the building, and use envd in the runtime, there may be different cache locations between build time and runtime.

For example, pip cache in build time will be stored in /home/root/.pip, and it should be /home/envd/.pip in runtime.

I am not sure if it is possible to customise the cache location.

Signed-off-by: Keming <kemingyang@tensorchord.ai>
@gaocegege
Copy link
Member

Please rebase.

Signed-off-by: Keming <kemingyang@tensorchord.ai>
Signed-off-by: Keming <kemingyang@tensorchord.ai>
@gaocegege
Copy link
Member

Signed-off-by: Keming <kemingyang@tensorchord.ai>
Signed-off-by: Keming <kemingyang@tensorchord.ai>
@kemingy
Copy link
Member Author

kemingy commented Oct 3, 2022

According to my observation, micromamba update -f env.yaml doesn't install anything from the YAML file. Instead, it only updates all the existing packages.

We'd better use conda update because it requires mounts, which means it cannot be cached.

Signed-off-by: Keming <kemingyang@tensorchord.ai>
@kemingy
Copy link
Member Author

kemingy commented Oct 3, 2022

I removed most of the chown operations. Users can still access conda, use pip install.

@kemingy kemingy changed the title feat: conda/mamba create env from env.yaml file refact: conda/mamba create/update env, fix user permissions Oct 3, 2022
@kemingy
Copy link
Member Author

kemingy commented Oct 3, 2022

https://open-vsx.org/api is too fragile. Sadly, GitHub actions don't support allow-failure actions/runner#2347.

Signed-off-by: Keming <kemingyang@tensorchord.ai>
@gaocegege
Copy link
Member

/lgtm

Thanks for your contribution! 🎉 👍

@muniu-bot muniu-bot bot added the lgtm label Oct 4, 2022
@kemingy kemingy merged commit bf993e2 into tensorchord:main Oct 4, 2022
@kemingy kemingy deleted the conda_create_env branch October 4, 2022 02:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants