-
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 map
with list
#2113
Closed
Closed
Changes from all commits
Commits
Show all changes
45 commits
Select commit
Hold shift + click to select a range
e1c03aa
Change status_history from map to list
jopemachine 59f4b60
Fix test
jopemachine f576ca5
Try to fix CI
jopemachine 42d12ed
Fix test
jopemachine 49338b6
Rename `sql_list_append` -> `sql_append_lists_to_list`
jopemachine 19bb91d
Add fragment
jopemachine da24de4
Fix wrong comment position
jopemachine feec035
Rename get_first_status_history_record function
jopemachine 380908a
Fix wrong implementation of session_history of vfolder
jopemachine 1f521a4
Add default value to session status history
jopemachine 2373005
Code organization
jopemachine 8f7a6d6
Rename get_first_occurrence_time function
jopemachine 9b0e4be
Add comments for status_history column
jopemachine 2cf840e
Try to fix _fetch_hanging_sessions
jopemachine 185164b
Fix broken _fetch_hanging_sessions
jopemachine f92a54a
Remove useless newline
jopemachine 9a6e0ad
Add migration script
jopemachine f0bf3f5
Change status history format
jopemachine 4cb68de
Add FieldSpec for status_history
jopemachine 5e44e85
Fix status_history command
jopemachine 1839ef3
Update obsoleted comments
jopemachine 1c26e4b
Update migration script
jopemachine 32dc1cc
Update _fetch_hanging_sessions SQL
jopemachine ddfce33
Allow to search stale sessions
jopemachine 76cc98e
Resolve alembic conflict
jopemachine b1520c9
chore: Merge with main
jopemachine 10f36b2
chore: update GraphQL schema dump
jopemachine 4b0758c
fix: revert unrelated change
jopemachine 240ee6c
docs: update migration script msg
jopemachine d0dc961
chore: Add description field
jopemachine 855f91f
chore: update GraphQL schema dump
jopemachine 2bb11f3
fix: alembic migration script down_revision
jopemachine 4080505
fix: Wrong handling of `get_first_occurrence_time`
jopemachine cc7d896
chore: Rename `get_first_occurrence_time` -> `get_first_timestamp_for…
jopemachine cf5d7d5
docs: Update fragment
jopemachine f25bf61
docs: Add `deprecation_reason` to `status_history` in gql
jopemachine 62237a3
chore: update GraphQL schema dump
jopemachine d89a64f
fix: Mishandling of `scheduled_at` in gql
jopemachine 6d4bc09
fix: Change `get_first_timestamp_for_status` return type to datetime
jopemachine 3af5ec9
fix: perform missing alter column in migration script
jopemachine 895fca8
refactor: Move get_first_timestamp_for_status() from common.utils to …
achimnol bf0f5a4
fix: Reconcile the alembic migration history
achimnol 4f63ea7
fix: Let migrations convert columns with the other type only
achimnol 3bb3d31
fix: Change data types that were mistakenly converted to tuples
jopemachine 5eeb502
fix: Update the codes added in #2275
achimnol 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: 59a622c31820 | ||
Create Date: 2024-01-26 11:19:23.075014 | ||
|
||
""" | ||
|
||
from alembic import op | ||
|
||
# revision identifiers, used by Alembic. | ||
revision = "8c8e90aebacd" | ||
down_revision = "59a622c31820" | ||
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) | ||
) | ||
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) | ||
) | ||
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;") |
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.
You've probably kept the existing behavior based on
dict
, but since you've changed the structure based onlist
, shouldn't you show the whole history change?