8000 Add config.owner to v1 syntax for setting UID and GID by aseaday · Pull Request #1286 · tensorchord/envd · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Add config.owner to v1 syntax for setting UID and GID #1286

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Dec 10, 2022
Merged

Add config.owner to v1 syntax for setting UID and GID #1286

merged 2 commits into from
Dec 10, 2022

Conversation

aseaday
Copy link
Member
@aseaday aseaday commented Dec 8, 2022 •
edited
Loading

This is a extra pr for #410.
There three ways to set UID/GID during building:

  1. read args from cli
  2. config.owner in build.envd
  3. get current user's uid/gid
    and their priorities.

@aseaday aseaday requested a review from zwpaper as a code owner December 8, 2022 15:41
gaocegege
gaocegege previously approved these changes Dec 9, 2022
View reviewed changes
Copy link
Member
@gaocegege gaocegege left a comment

Choose a reason for hiding this comment

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

/cc @kemingy

LGTM

@muniu-bot muniu-bot bot requested a review from kemingy December 9, 2022 01:18
kemingy
kemingy reviewed Dec 9, 2022
View reviewed changes
@aseaday
Change gid arg to be necessary
ea381cb 
Signed-off-by: nullday <aseaday@hotmail.com>
kemingy
kemingy approved these changes Dec 9, 2022
View reviewed changes
Copy link
Member
@kemingy kemingy left a comment

Choose a reason for hiding this comment

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

LGTM.

Shall we support this feature in v0? @gaocegege

@gaocegege
Copy link
Member
gaocegege commented Dec 9, 2022 •
edited
Loading

Let's merge it and see. I think there is no need to support for v0 now. WDYT

@aseaday
Copy link
Member Author
aseaday commented Dec 9, 2022

I don't think we need to support it in v1. I think all v0 update from today should be only bug fix or critical issues.

@aseaday aseaday closed this Dec 9, 2022
@aseaday aseaday reopened this Dec 9, 2022
@gaocegege
Copy link
Member

/lgtm
/approve

@muniu-bot muniu-bot bot added the lgtm label Dec 9, 2022
@kemingy kemingy merged commit f7f2b34 into tensorchord:main Dec 10, 2022
AlexXi19 pushed a commit to AlexXi19/envd that referenced this pull request Dec 12, 2022
@aseaday @AlexXi19
Add config.owner to v1 syntax for setting UID and GID (tensorchord#1286)
e0f7cf6 
* Add config.owner to v1

* Change gid arg to be necessary

Signed-off-by: nullday <aseaday@hotmail.com>

Signed-off-by: nullday <aseaday@hotmail.com>
Signed-off-by: AlexXi19 <alex2001314jjj@gmail.com>
@kemingy kemingy mentioned this pull request Dec 13, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kemingy kemingy kemingy approved these changes

@gaocegege gaocegege gaocegege left review comments

@zwpaper zwpaper Awaiting requested review from zwpaper zwpaper is a code owner

Assignees

@gaocegege gaocegege

Labels
lgtm
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@aseaday @gaocegege @kemingy
0