-
Notifications
You must be signed in to change notification settings - Fork 293
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
Add docker rootless feature flag and its implementation for supporting docke rootless environment #818
Add docker rootless feature flag and its implementation for supporting docke rootless environment #818
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @kimsehwan96 👋 Thanks a lot for your contribution - do you think it would be possible to cover this by test by any chance, or it's gonna be rather hard to include in the testing suite?
Hello @pgrzesik . I think that this change should be tests in CI(Github action) level. Becasue it doesn't work that we expected when enable that feature flag in I will find how to test it in unit test or CI level test in github action. I think that it will not very hard to include this in the testing suite! |
Hey @kimsehwan96 👋 Could you fix the formatting as listed in the failed CI build? I think having automated tests for this would be tricky, but let's make sure it's not breaking anything else in the process 👍 |
Hello @pgrzesik . Sorry for late works.. As you mentioned, apply automated tests for this change is tricky. I underestimated about it 🥲. And I failed to devote much of time. By the way, I will applying the formatting and inform about it to you. When I have some free time from the coming week, I will append some comment about which cases of environment this will be needed with some detailed information. Thank you ! |
d77d6e6
to
53d76b0
Compare
I applied formatting for this change ! @pgrzesik I want to merge 2 commits which are for applying |
…porting docke rootless environment
53d76b0
to
aa3b05e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @kimsehwan96 🙇
…g docke rootless environment (serverless#818)
resolve: #817
Add feature flag
dockerRootless
and default valuefalse
.Change
lib/pip.js
file permission change logic for supporting docker rootless environment.I think that its better add some document about this in
README.md