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

ilab-wrapper: Improve non-single subuid ranges error handling #730

Merged
merged 1 commit into from
Aug 7, 2024

Conversation

omertuc
Copy link
Contributor

@omertuc omertuc commented Aug 6, 2024

Background

df88857

Issue

The current error handling for multiple subuid ranges is broken due to surprising behavior of wc -l which always returns 1 even when the input is empty.

Solution

More carefully count the number of lines in the CURRENT_USER_SUBUID_RANGE variable

Additional changes

omertuc@50fb00f had a small merge error, this commit fixes that.

training/ilab-wrapper/ilab Outdated Show resolved Hide resolved
@eranco74
Copy link
Contributor

eranco74 commented Aug 6, 2024

/lgtm
/approve

@omertuc omertuc force-pushed the bash branch 3 times, most recently from fd4060c to 4118da9 Compare August 7, 2024 08:45
@rhatdan
Copy link
Member

rhatdan commented Aug 7, 2024

Needs a rebase or to be closed, it the code was already merged.

@omertuc omertuc force-pushed the bash branch 2 times, most recently from 926082d to 367b116 Compare August 7, 2024 10:35
@omertuc
Copy link
Contributor Author

omertuc commented Aug 7, 2024

Force pushed to fix conflicts

training/ilab-wrapper/ilab Outdated Show resolved Hide resolved
# Background

df88857

# Issue

The current error handling for multiple subuid ranges is broken due to
surprising behavior of `wc -l` which always returns `1` even when the
input is empty.

# Solution

More carefully count the number of lines in the
`CURRENT_USER_SUBUID_RANGE` variable

# Additional changes

50fb00f had a small merge error, this
commit fixes that.

Signed-off-by: Omer Tuchfeld <[email protected]>
@rhatdan
Copy link
Member

rhatdan commented Aug 7, 2024

LGTM

@rhatdan rhatdan merged commit 04bccc6 into containers:main Aug 7, 2024
1 check passed
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

Successfully merging this pull request may close these issues.

3 participants