feat: Add the ability to override resources on a per-user basis #1981
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The generated OpenAPI client is not up to date with the latest changes in the OpenAPI specification. |
API Changelog 4.15.1.dev18+g39fabe1fGET /api/v1/projects/{project_slug}/models
POST /api/v1/projects/{project_slug}/models
GET /api/v1/projects/{project_slug}/models/{model_slug}
PATCH /api/v1/projects/{project_slug}/models/{model_slug}
GET /api/v1/projects/{project_slug}/models/{model_slug}/provisioning
GET /api/v1/projects/{project_slug}/tools
POST /api/v1/projects/{project_slug}/tools
GET /api/v1/sessions
POST /api/v1/sessions
GET /api/v1/sessions/{session_id}
GET /api/v1/tools
POST /api/v1/tools
GET /api/v1/tools/*/versions
GET /api/v1/tools/default
GET /api/v1/tools/{tool_id}
PUT /api/v1/tools/{tool_id}
GET /api/v1/users/{user_id}/sessions
|
This comment has been minimized.
This comment has been minimized.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1981 +/- ##
==========================================
+ Coverage 85.46% 85.53% +0.07%
==========================================
Files 225 225
Lines 7411 7448 +37
Branches 513 524 +11
==========================================
+ Hits 6334 6371 +37
+ Misses 913 912 -1
- Partials 164 165 +1 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
zusorio
force-pushed
the
custom-resources-for-users
branch
from
c773244 to
cab1a6a
Compare
This comment has been minimized.
This comment has been minimized.
|
A Storybook preview is available for commit 39fabe1. |
zusorio
force-pushed
the
custom-resources-for-users
branch
from
cab1a6a to
ee32de9
Compare
This comment has been minimized.
This comment has been minimized.
zusorio
force-pushed
the
custom-resources-for-users
branch
from
ee32de9 to
913c962
Compare
This comment has been minimized.
This comment has been minimized.
zusorio
force-pushed
the
custom-resources-for-users
branch
from
913c962 to
7f8a7f0
Compare
zusorio
force-pushed
the
custom-resources-for-users
branch
2 times, most recently
from
7c78706 to
dee00aa
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
zusorio
force-pushed
the
custom-resources-for-users
branch
from
dee00aa to
6faaae9
Compare
This comment has been minimized.
This comment has been minimized.
zusorio
force-pushed
the
custom-resources-for-users
branch
from
6faaae9 to
bccb9e0
Compare
This comment has been minimized.
This comment has been minimized.
zusorio
force-pushed
the
custom-resources-for-users
branch
from
bccb9e0 to
c7638a7
Compare
This comment has been minimized.
This comment has been minimized.
zusorio
force-pushed
the
custom-resources-for-users
branch
from
c7638a7 to
36301ab
Compare
This comment has been minimized.
This comment has been minimized.
zusorio
force-pushed
the
custom-resources-for-users
branch
from
36301ab to
5511b3e
Compare
This comment has been minimized.
This comment has been minimized.
zusorio
force-pushed
the
custom-resources-for-users
branch
from
5511b3e to
bd2222e
Compare
This comment has been minimized.
This comment has been minimized.
zusorio
force-pushed
the
custom-resources-for-users
branch
from
bd2222e to
c65e4ac
Compare
This comment has been minimized.
This comment has been minimized.
zusorio
force-pushed
the
custom-resources-for-users
branch
2 times, most recently
from
978e1df to
b821a01
Compare
This comment has been minimized.
This comment has been minimized.
MoritzWeber0
requested changes
Dec 2, 2024
Comment on lines
+254
to
+259
| default_profile: DefaultResourceProfile = pydantic.Field( | ||
| default_factory=DefaultResourceProfile, | ||
| description="Default resource profile, which is used when no other profile matches.", | ||
| ) | ||
| additional: dict[str, AdditionalResourceProfile] = pydantic.Field( | ||
| default={}, | ||
| description="Additional resource profiles, which can be used to limit the resource usage of sessions.", | ||
| ) |
There was a problem hiding this comment.
I'd add another layer in the configuration:
resources:
profiles:
default: ...
additional: ...If you change it, make sure to also update the docs and migration script.
| ) | ||
|
|
||
|
|
||
| class Resources(core_pydantic.BaseModelStrict): |
There was a problem hiding this comment.
The API Changelog generation indicates for which endpoints the class is used: #1981 (comment)
For data protection and political reasons, we should not expose the list of usernames of the profiles to users with role user. We should hide the list of usernames in the response for normal users (e.g., via pydantic validator). Alternatively, we can return only the relevant resource profile for the user.
Comment on lines
+73
to
+85
| tools = get_tools(db) | ||
| for tool in tools: | ||
| updated = False | ||
| for profile in tool.config.resources.additional.values(): | ||
| if old_username in profile.usernames: | ||
| profile.usernames = [ | ||
| new_username if username == old_username else username | ||
| for username in profile.usernames | ||
| ] | ||
| updated = True | ||
| if updated: | ||
| orm.attributes.flag_modified(tool, "config") | ||
| db.commit() |
There was a problem hiding this comment.
Suggested change
| tools = get_tools(db) | |
| for tool in tools: | |
| updated = False | |
| for profile in tool.config.resources.additional.values(): | |
| if old_username in profile.usernames: | |
| profile.usernames = [ | |
| new_username if username == old_username else username | |
| for username in profile.usernames | |
| ] | |
| updated = True | |
| if updated: | |
| orm.attributes.flag_modified(tool, "config") | |
| db.commit() | |
| tools = get_tools(db) | |
| for tool in tools: | |
| for profile in tool.config.resources.additional.values(): | |
| if old_username in profile.usernames: | |
| profile.usernames = [ | |
| new_username if username == old_username else username | |
| for username in profile.usernames | |
| ] | |
| orm.attributes.flag_modified(tool, "config") | |
| db.commit() |
|
|
||
| @pydantic.field_validator("additional") | ||
| @classmethod | ||
| def check_additional_profiles( |
There was a problem hiding this comment.
A validation if the user exists is missing.
| @@ -80,6 +81,8 @@ def create_user( | |||
| def update_user( | |||
There was a problem hiding this comment.
If the user is deleted, the username should be removed from the list.
zusorio
force-pushed
the
custom-resources-for-users
branch
from
b821a01 to
3b05699
Compare
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #1951