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

feat: Add API to get available resources #442

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

Conversation

romanceformoon
Copy link
Contributor

lablup/backend.ai#249

This PR adds API to get available resources in groups.
Users can get available resources by API GET method.

@CLAassistant
Copy link

CLAassistant commented Jul 5, 2021

CLA assistant check
All committers have signed the CLA.

@codecov
Copy link

codecov bot commented Jul 5, 2021

Codecov Report

Merging #442 (0805344) into main (27d9325) will decrease coverage by 0.12%.
The diff coverage is 34.21%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #442      +/-   ##
==========================================
- Coverage   48.28%   48.16%   -0.13%     
==========================================
  Files          52       52              
  Lines        8708     8763      +55     
==========================================
+ Hits         4205     4221      +16     
- Misses       4503     4542      +39     
Impacted Files Coverage Δ
src/ai/backend/manager/server.py 60.98% <ø> (ø)
src/ai/backend/manager/models/resource_preset.py 60.34% <34.21%> (-12.74%) ⬇️
src/ai/backend/manager/idle.py 30.84% <0.00%> (-0.81%) ⬇️

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 27d9325...0805344. Read the comment docs.

@romanceformoon romanceformoon changed the title release: Add API to get available resources feat: Add API to get available resources Jul 6, 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.

  • Let's refactor the common reusable parts from get_available_resources() and the above existing check_presets() handler as a shared function in the ai.backend.models.resource_preset module, just like query_allowed_sgroups().
  • Though it seems that group resource checks are not implemented yet, please apply the group resource visibility option when you do.

@@ -748,6 +856,7 @@ def create_app(default_cors_options: CORSOptions) -> Tuple[web.Application, Iter
add_route = app.router.add_route
cors.add(add_route('GET', '/presets', list_presets))
cors.add(add_route('POST', '/check-presets', check_presets))
cors.add(add_route('GET', '/get-resources', get_available_resources))
Copy link
Member

Choose a reason for hiding this comment

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

Let's rename the URL to /available-resources since the HTTP method name "GET" already indicates its a query API.

@achimnol achimnol added this to the 21.09 milestone Jul 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.

Please add type annotations as much as possible when you write new functions.

@@ -193,3 +212,88 @@ async def mutate(
.where(resource_presets.c.name == name)
)
return await simple_db_mutate(cls, info.context, delete_query)


async def get_row(conn, request, params, domain_name):
Copy link
Member

Choose a reason for hiding this comment

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

Please clarify the name of this function. What is the row? Where is it from?

romanceformoon and others added 5 commits July 16, 2021 16:52
…rce prefix

* WIP: rewriting resource check routines -- but we need to discuss the policy about
  visibility of scaling group capacity and the sum of other user's occupation to non-admin users,
  because these new APIs directly expose the exact amount of resources unlike the
  check-preset API.
* fix: Apply consistent API logging headers (e.g., LIST_RESETS -> RESORUCE.LIST_PRESETS)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants