Skip to content
This repository has been archived by the owner on Sep 22, 2023. It is now read-only.

feat: Display kernel-pull-progress #181

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

youngjun0627
Copy link

@youngjun0627 youngjun0627 commented Sep 1, 2021

backend.ai-issue#227

Client (Python & Javascript) Task

  • Check the existence of the task ID in the create-kernel API response.

  • If it exists, show a progress bar reflecting its progress stream.

Using response from manager, client obtains background-task pulling kernel and displays progress bar reflecting kernel-pulling. But, I do request twice.

  • From first request, client obtains kernel_id and background task.

  • From second request, client gets all the information from existed session.

But, I think we need another way to get all the information(contains background-task id) from manager with a single request.

@youngjun0627 youngjun0627 changed the title feat: display kernel-pull-progress feat: Display kernel-pull-progress Sep 1, 2021
@youngjun0627 youngjun0627 force-pushed the feat/display-kernelpullprogress branch 4 times, most recently from fa7934a to ee2851d Compare September 6, 2021 05:55
@youngjun0627 youngjun0627 force-pushed the feat/display-kernelpullprogress branch from ee2851d to f7fafe5 Compare September 6, 2021 15:08
@achimnol achimnol added this to the 21.09 milestone Sep 8, 2021
Copy link
Member

@achimnol achimnol left a comment

Choose a reason for hiding this comment

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

Copy link
Member

@achimnol achimnol left a comment

Choose a reason for hiding this comment

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

The client.func module provides a set of functional API wrappers while client.cli module provides the CLI-specific argument parsing and outputs.
So, could we move the invocation to tqdm to the cli module? Of course, the bgtask handling routines must stay here, so we need something like "reporter" interface as in the manager to separate the console output handling and the abstract bgtask handling.

@codecov
Copy link

codecov bot commented Sep 9, 2021

Codecov Report

Merging #181 (b4520c3) into main (b6a2bd9) will decrease coverage by 0.12%.
The diff coverage is 18.18%.

❗ Current head b4520c3 differs from pull request most recent head b93a492. Consider uploading reports for the commit b93a492 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##             main     #181      +/-   ##
==========================================
- Coverage   45.46%   45.34%   -0.13%     
==========================================
  Files          74       74              
  Lines        6801     6815      +14     
==========================================
- Hits         3092     3090       -2     
- Misses       3709     3725      +16     
Impacted Files Coverage Δ
src/ai/backend/client/func/session.py 42.06% <18.18%> (-2.32%) ⬇️
src/ai/backend/client/cli/params.py 39.47% <0.00%> (+0.34%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b6a2bd9...b93a492. Read the comment docs.

@youngjun0627
Copy link
Author

When status of the ComputeSession is 'TIMEOUT' or 'PENDING', client prints the info(logs) and return in current code. So, will I implement the code by your feedback in this situations?

@achimnol
Copy link
Member

When status of the ComputeSession is 'TIMEOUT' or 'PENDING', client prints the info(logs) and return in current code. So, will I implement the code by your feedback in this situations?

Yes, there was no way to keep track of the kernel preparation process before, so we just printed a messaged and exited.
Now let it use the bgtask API to show the progress and preparation status afterwards upon TIMEOUT or PENDING.

@youngjun0627
Copy link
Author

youngjun0627 commented Sep 10, 2021

  • idea
  1. func/session.py
    Check if the response from manager has 'background_task' key. If True, the Compute-Session has a attribute 'backgroundtask' and is returned.
  2. cli/run.py
    When the compute-session' status is 'TIMEOUT' or 'PENDING', display the progress using bgtask-handler. And if background task is done, the compute-session calls 'get_or_created' method containing two arguments 'image', 'name' to get(update?) its attributions.

But, by your comment, background task handler stays func/session.py and the reporter attribution is added to compute-session? I tried it but in this case, get_or_create method returns after background task is finished. Do I misunderstand?

@youngjun0627 youngjun0627 force-pushed the feat/display-kernelpullprogress branch 2 times, most recently from 41a3d7e to 8380a5c Compare September 12, 2021 06:05
@youngjun0627 youngjun0627 force-pushed the feat/display-kernelpullprogress branch 2 times, most recently from 7e8809e to 171918b Compare September 12, 2021 09:56
@youngjun0627 youngjun0627 force-pushed the feat/display-kernelpullprogress branch from 171918b to b93a492 Compare September 12, 2021 10:01
@achimnol achimnol assigned achimnol and unassigned achimnol Nov 25, 2021
@achimnol achimnol self-assigned this Dec 14, 2021
@mindgitrwx mindgitrwx self-assigned this Dec 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants