feat: Replace sessions, kernels's status_history's type map with list
#2113
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
Your org has enabled the Graphite merge queue for merging into mainAdd the label “flow:merge-queue” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge. Or use the label “flow:hotfix” to add to the merge queue as a hot fix. You must have a Graphite account and log in to Graphite in order to use the merge queue. Sign up using this link. |
github-actions
bot
added
area:docs
2 tasks
This stack of pull requests is managed by Graphite. Learn more about stacking. |
2 tasks
jopemachine
force-pushed
the
topic/05-07-refactor_status_history
branch
from
222dd6d to
24dfca2
Compare
achimnol
force-pushed
the
topic/05-07-refactor_status_history
branch
3 times, most recently
from
5495f32 to
b707634
Compare
achimnol
force-pushed
the
topic/05-07-refactor_status_history
branch
2 times, most recently
from
2b525ff to
2d3e2b4
Compare
…manager.models.utils - This eliminates the necessity to use stringified enum arguments in the `ai.backend.manager.models` codes. - In the client SDK, add a copy of it using relaxed str-only arguments. Since this is a fairly simple logic, I think it is not worth to introduce additional complexity to share and reuse the code between the client SDK and the manager. (Note that originally `ai.backend.common` was not the dependency of the client SDK...) - I think it would be better to introduce a JSON-fied TypedDict of each status history records. - Also fix several merge errors.
achimnol
force-pushed
the
topic/05-07-refactor_status_history
branch
from
2d3e2b4 to
5eeb502
Compare
Comment on lines
+814
to
+823
| if (preparing := get_first_timestamp_for_status(status_history, "PREPARING")) is None: | ||
| elapsed = timedelta() | ||
| elif ( | ||
| terminated := get_first_timestamp_for_status(status_history, "TERMINATED") | ||
| ) is None: | ||
| elapsed = datetime.now(tzutc()) - preparing | ||
| else: | ||
| alloc_time: timedelta = isoparse(terminated) - isoparse(preparing) | ||
| result = { | ||
| "result": { | ||
| "seconds": alloc_time.seconds, | ||
| "microseconds": alloc_time.microseconds, | ||
| }, | ||
| } | ||
| print_done(f"Actual Resource Allocation Time: {result}") | ||
| elapsed = terminated - preparing | ||
|
|
||
| print_done(f"Actual Resource Allocation Time: {elapsed.total_seconds()}") |
There was a problem hiding this comment.
You've probably kept the existing behavior based on dict, but since you've changed the structure based on list, shouldn't you show the whole history change?
jopemachine
changed the title
sessions, kernels's status_history's type map with listsessions, kernels's status_history's type map with list
jopemachine
added
type:refactor
|
Since this PR adds or modifies the API, it has been changed to "feat". |
|
This original PR has become too outdated, requiring excessive effort to update, so a new PR has been created. |
2 tasks
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.
Refs #412, follow-up to #480.
Related PR: #1116.
The current implementation saves only the most recent timestamp whenever status information in
status_historyis updated, and all previous information is deleted.This PR prevents the loss of timestamp information by changing the data structure of
sessions,kernels'sstatus_historyto List.Previous format:
New format:
Checklist: (if applicable)
📚 Documentation preview 📚: https://sorna--2113.org.readthedocs.build/en/2113/
📚 Documentation preview 📚: https://sorna-ko--2113.org.readthedocs.build/ko/2113/