Conversation
Signed-off-by: Esteban Aguililla Klein <esteban.aguililla.klein.pro@outlook.com>
Signed-off-by: Esteban Aguililla Klein <esteban.aguililla.klein.pro@outlook.com>
apaolillo
left a comment
apaolillo
left a comment
There was a problem hiding this comment.
Thanks @EstAK . It is a valuable feature. However, my overall feeling is that this PR does too many changes at once. It should probably split the user manager from the context manager, and the scala example could also come later (you may do a smaller example to validate the changes). Also is this change breaking the API? We can further discuss this.
| tailored specifically for Docker environments. | ||
| """ | ||
|
|
||
| from __future__ import annotations |
| """ | ||
|
|
||
| def __init__( | ||
| self, partial_docker_builder: PartialDockerBuilder, on_behalf: str, return_to: str |
There was a problem hiding this comment.
| self, partial_docker_builder: PartialDockerBuilder, on_behalf: str, return_to: str | |
| self, partial_docker_builder: PartialDockerBuilder, on_behalf: str, return_to: str, |
and apply format (style.sh)
| """ | ||
| A class that implements a user context manager where within said context, all actions will be | ||
| done on behalf of a chosen user. | ||
| """ |
| self.partial_docker_builder.user(name=self.on_behalf) | ||
| return self.partial_docker_builder | ||
|
|
||
| def __exit__(self, exc_type, exc_value, traceback): |
There was a problem hiding this comment.
| def __exit__(self, exc_type, exc_value, traceback): | |
| def __exit__(self, exc_type, exc_value, traceback) -> None: |
|
|
||
| def manage_user(self, username: str) -> None: | ||
| """ | ||
| Manages the user `username` through the `UserManager` |
There was a problem hiding this comment.
I don't understand what does that mean.
| """ | ||
| if not name: | ||
| name = "${USER_NAME}" # fallback user | ||
| elif check and (name not in self.managed_users): |
There was a problem hiding this comment.
not sure why this check is necessary
| partial_docker_builder._build_commands.append(StrDockerBuildCommand(cmd)) | ||
|
|
||
| def as_user( | ||
| self, partial_docker_builder: PartialDockerBuilder, username: str |
There was a problem hiding this comment.
| self, partial_docker_builder: PartialDockerBuilder, username: str | |
| self, partial_docker_builder: PartialDockerBuilder, username: str, |
| return_to: str = self.current_user | ||
| self.current_user = username | ||
|
|
||
| return UserContext(partial_docker_builder, on_behalf=self.current_user, return_to=return_to) |
There was a problem hiding this comment.
| return UserContext(partial_docker_builder, on_behalf=self.current_user, return_to=return_to) | |
| return UserContext(partial_docker_builder, on_behalf=self.current_user, return_to=return_to,) |
| Initializes a PartialDockerBuilder with an empty list of build commands. | ||
| """ | ||
| self._build_commands: List[DockerBuildCommand] = [] | ||
| self.user_manager: UserManager | None = None |
There was a problem hiding this comment.
this means we maintain in the builder all the users that were created?
src/pythainer/builders/__init__.py
Outdated
| command = " && \\\n ".join(commands) | ||
| self.run(command=command) | ||
|
|
||
| def user(self, name: str = "") -> None: |
There was a problem hiding this comment.
I'd like to keep the feature as before as well. Sometimes you don't want to return you just want a one way USER change.
|
as discussed: we split into more incremental feature set. |
2 participants
Features
UserManagerMotivation
Instead of trusting the user to correctly scope where a user should be enable : encapsulate it. No need to use constructs like anymore :
now, you can use
at the end of the context manager you are ensured to return to the user that was used before entering it :