-
Notifications
You must be signed in to change notification settings - Fork 157
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
feat: Replace sessions
, kernels
's status_history
's type dict
with list
#3201
Open
jopemachine
wants to merge
16
commits into
main
Choose a base branch
from
topic/12-05-feat_replace_sessions_kernels_s_status_history_s_type_dict_with_list_
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
ed3e4f0
feat: Replace `sessions`, `kernels`'s `status_history`'s type `dict` …
jopemachine 17277a0
fix: Use SessionId, from_session_id
jopemachine dcd9172
chore: Rename news fragment
jopemachine 228e5b0
chore: fix todo comment
jopemachine 7634151
refactor: Use async `cmd_main`
jopemachine e823c6f
fix: Remove useless list copy
jopemachine 1699c22
fix: Outdated KernelNode.from_row
jopemachine 160b323
fix: Outdated ComputeSessionNode.from_row
jopemachine a6354dd
fix: Missing status_history field
jopemachine b983e59
fix: Wrong import
jopemachine e2c0139
fix: Broken scheduled_at field in queryfilter
jopemachine c2209fa
chore: update GraphQL schema dump
jopemachine b1522d0
docs: Rename the news fragment with the PR number\n\n.feature.md -> 3…
Yaminyam 2b58767
fix: typos
jopemachine dfa5f24
chore: Rename news fragment
jopemachine 8824526
fix: alembic multiple head
jopemachine File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
Change the type of `status_history` from a mapping of status and timestamps to a list of log entries containing status and timestamps, to preserve timestamps when revisiting session/kernel statuses (e.g., after session restarts). |
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
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
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
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
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
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
109 changes: 109 additions & 0 deletions
109
...ai/backend/manager/models/alembic/versions/8c8e90aebacd_replace_status_history_to_list.py
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,109 @@ | ||
"""Replace sessions, kernels's status_history's type map with list | ||
Revision ID: 8c8e90aebacd | ||
Revises: 0bb88d5a46bf | ||
Create Date: 2024-12-05 11:19:23.075014 | ||
""" | ||
|
||
from alembic import op | ||
|
||
# revision identifiers, used by Alembic. | ||
revision = "8c8e90aebacd" | ||
down_revision = "0bb88d5a46bf" | ||
branch_labels = None | ||
depends_on = None | ||
|
||
|
||
def upgrade(): | ||
op.execute( | ||
""" | ||
WITH data AS ( | ||
SELECT id, | ||
(jsonb_each(status_history)).key AS status, | ||
(jsonb_each(status_history)).value AS timestamp | ||
FROM kernels | ||
WHERE jsonb_typeof(status_history) = 'object' | ||
) | ||
UPDATE kernels | ||
SET status_history = ( | ||
SELECT jsonb_agg( | ||
jsonb_build_object('status', status, 'timestamp', timestamp) | ||
ORDER BY timestamp | ||
) | ||
FROM data | ||
WHERE data.id = kernels.id | ||
AND jsonb_typeof(kernels.status_history) = 'object' | ||
); | ||
""" | ||
) | ||
op.execute("UPDATE kernels SET status_history = '[]'::jsonb WHERE status_history IS NULL;") | ||
op.alter_column("kernels", "status_history", nullable=False, default=[]) | ||
|
||
op.execute( | ||
""" | ||
WITH data AS ( | ||
SELECT id, | ||
(jsonb_each(status_history)).key AS status, | ||
(jsonb_each(status_history)).value AS timestamp | ||
FROM sessions | ||
WHERE jsonb_typeof(status_history) = 'object' | ||
) | ||
UPDATE sessions | ||
SET status_history = ( | ||
SELECT jsonb_agg( | ||
jsonb_build_object('status', status, 'timestamp', timestamp) | ||
ORDER BY timestamp | ||
) | ||
FROM data | ||
WHERE data.id = sessions.id | ||
AND jsonb_typeof(sessions.status_history) = 'object' | ||
); | ||
""" | ||
) | ||
op.execute("UPDATE sessions SET status_history = '[]'::jsonb WHERE status_history IS NULL;") | ||
op.alter_column("sessions", "status_history", nullable=False, default=[]) | ||
|
||
|
||
def downgrade(): | ||
op.execute( | ||
""" | ||
WITH data AS ( | ||
SELECT id, | ||
jsonb_object_agg( | ||
elem->>'status', elem->>'timestamp' | ||
) AS new_status_history | ||
FROM kernels, | ||
jsonb_array_elements(status_history) AS elem | ||
WHERE jsonb_typeof(status_history) = 'array' | ||
GROUP BY id | ||
) | ||
UPDATE kernels | ||
SET status_history = data.new_status_history | ||
FROM data | ||
WHERE data.id = kernels.id | ||
AND jsonb_typeof(kernels.status_history) = 'array'; | ||
""" | ||
) | ||
op.alter_column("kernels", "status_history", nullable=True, default=None) | ||
op.execute("UPDATE kernels SET status_history = NULL WHERE status_history = '[]'::jsonb;") | ||
|
||
op.execute( | ||
""" | ||
WITH data AS ( | ||
SELECT id, | ||
jsonb_object_agg( | ||
elem->>'status', elem->>'timestamp' | ||
) AS new_status_history | ||
FROM sessions, | ||
jsonb_array_elements(status_history) AS elem | ||
WHERE jsonb_typeof(status_history) = 'array' | ||
GROUP BY id | ||
) | ||
UPDATE sessions | ||
SET status_history = data.new_status_history | ||
FROM data | ||
WHERE data.id = sessions.id | ||
AND jsonb_typeof(sessions.status_history) = 'array'; | ||
""" | ||
) | ||
op.alter_column("sessions", "status_history", nullable=True, default=None) | ||
op.execute("UPDATE sessions SET status_history = NULL WHERE status_history = '[]'::jsonb;") |
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
Oops, something went wrong.
Oops, something went wrong.
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.
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.
I don't know much about
session.command
, but can you subtract the internal string "status-history"?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.
backend.ai/src/ai/backend/client/cli/session/lifecycle.py
Lines 59 to 61 in 42b4030
Looking at the code, it seems that the decorator itself does not perform any operations on the arguments.
So, I think it would be better to leave the arguments empty, as is currently done in most of the other code.
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.
I think this is using
overload
to overload the function for the last implementation, can you double check? @jopemachineThere 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.
If no value is passed as an argument, the decorated function name is automatically used as the CLI command.
If we pass an arbitrary string, that string can be used as the CLI command instead of changing the function name.
So, let's leave the arguments empty here, just like with the other functions.