Skip to content
This repository has been archived by the owner on Aug 2, 2023. It is now read-only.

Add support for dotfiles of domains and groups #329

Merged
merged 19 commits into from
Sep 21, 2020

Conversation

yongkyunlee
Copy link
Contributor

  • Add dotfiles column for tables domains and groups.
  • Add manager API endpoints for listing/getting, creating, updating, and deleting dotfiles.

Solving issue lablup/backend.ai#210.

@codecov
Copy link

codecov bot commented Sep 11, 2020

Codecov Report

Merging #329 into master will decrease coverage by 0.12%.
The diff coverage is 36.36%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #329      +/-   ##
==========================================
- Coverage   48.97%   48.85%   -0.13%     
==========================================
  Files          47       47              
  Lines        6995     7058      +63     
==========================================
+ Hits         3426     3448      +22     
- Misses       3569     3610      +41     
Impacted Files Coverage Δ
src/ai/backend/gateway/server.py 57.69% <ø> (ø)
src/ai/backend/manager/registry.py 20.02% <0.00%> (-0.60%) ⬇️
src/ai/backend/manager/models/group.py 45.86% <45.45%> (-0.04%) ⬇️
src/ai/backend/manager/models/domain.py 58.57% <52.63%> (-1.03%) ⬇️
src/ai/backend/gateway/exceptions.py 86.05% <100.00%> (+0.20%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ad5e971...3a57821. Read the comment docs.

@yongkyunlee yongkyunlee marked this pull request as draft September 14, 2020 06:52
@yongkyunlee yongkyunlee requested a review from adrysn September 14, 2020 09:36
Copy link
Member

@adrysn adrysn left a comment

Choose a reason for hiding this comment

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

I left some comments mostly on authorizations.

And, I am worried about what will happen when the name of a dotfile collides with the name of a virtual folder. For example, let's say a superadmin created a dotfile named .test, which will be mounted at /home/work/.test inside a session container. And there is a chance that some users have a virtual folder named .test, which will be auto-mounted. The user tries to launch a session. I'm not sure since I didn't test, but probably the session will not be launched due to naming conflict.

This is not very serious for user's dotfiles since the problem will arise only for that user. But, for domain and group dotfiles, the problem will affect many domains' and/or groups' users, if admins are careless with the dotfile naming.

Currently, I don't have a clear idea to prevent this problem. Let's discuss further with others.

@yongkyunlee yongkyunlee requested a review from adrysn September 16, 2020 08:35
@yongkyunlee yongkyunlee marked this pull request as ready for review September 16, 2020 08:35
@adrysn
Copy link
Member

adrysn commented Sep 21, 2020

LGTM. Please merge the master branch into this one, and resolve conflicting file(s). Also, try to fix broken tests if possible.

@yongkyunlee yongkyunlee merged commit 5cfb44b into master Sep 21, 2020
@yongkyunlee yongkyunlee deleted the feature/add-support-shell-profiles branch September 21, 2020 02:03
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants