Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix: Handle buggy orm field loading when destroy session #1312

Merged
merged 5 commits into from
Jun 5, 2023
Merged

fix: Handle buggy orm field loading when destroy session #1312

merged 5 commits into from
Jun 5, 2023

Conversation

fregataa
Copy link
Member

@fregataa fregataa commented May 30, 2023 •
edited
Loading

ORM objects must load its fields before access to them.
Else, we get DetachedInstanceError

File "/home/bai-23.03-manager/lib/python3.11/site-packages/ai/backend/manager/registry.py", line 1632, in destroy_session
      kernel.session_creation_id,
      ^^^^^^^^^^^^^^^^^

@fregataa fregataa added this to the 23.03 milestone May 30, 2023
@fregataa fregataa requested a review from adrysn May 30, 2023 05:40
@fregataa fregataa self-assigned this May 30, 2023
@github-actions github-actions bot added comp:manager Related to Manager component size:XS ~10 LoC labels May 30, 2023
adrysn
adrysn approved these changes May 30, 2023
View reviewed changes
Copy link
Member

@adrysn adrysn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@achimnol achimnol added this pull request to the merge queue Jun 5, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 5, 2023
@achimnol achimnol enabled auto-merge June 5, 2023 09:58
@achimnol
Copy link
Member

achimnol commented Jun 5, 2023 •
edited
Loading

The current test failure (https://github.com/lablup/backend.ai/actions/runs/5175780163/jobs/9323797688) looks like an issue from Pants: pantsbuild/pants#18519

@achimnol achimnol added this pull request to the merge queue Jun 5, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 5, 2023
@achimnol
fix: Ensure types-six is loaded along with graphql and promise
434f614 
- This resolves the bogus typecheck failure.
@achimnol achimnol enabled auto-merge June 5, 2023 12:36
@achimnol achimnol added this pull request to the merge queue Jun 5, 2023
Merged via the queue into main with commit 57779d7 Jun 5, 2023
@achimnol achimnol deleted the fix/handle-kernel-orm-field-loading-in-destroy-session branch June 5, 2023 12:41
achimnol added a commit that referenced this pull request Jun 5, 2023
@fregataa @achimnol
fix: Handle buggy orm field loading when destroy session (#1312)
2757a9e 
Co-authored-by: Joongi Kim <joongi@lablup.com>
Backported-from: main
Backported-to: 23.03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adrysn adrysn adrysn approved these changes

Assignees

@fregataa fregataa

Labels
comp:manager Related to Manager component size:XS ~10 LoC
Projects
None yet
Milestone
23.03
Development

Successfully merging this pull request may close these issues.
3 participants
@fregataa @achimnol @adrysn