feat: add `OptionalType` click parameter type #1406

kimjinmyeong · 2023-07-21T04:28:22Z

This PR resolves #1393 , and includes minor fixes to execute follow commands:

./backend.ai sesstpl create -f {template_filepath}
./backend.ai session create-from-template {template_id}

Implement OptionalType class

OptionalType serves as a type wrapper that checks for undefined types.

Revise to support for YAML format files and enable the creation of multiple templates.

To create multiple templates and iterate through a for loop when importing YAML file, the safe_load function should be replaced with the safe_load_all function.
To use the same code for creating query in both YAML and JSON formats, the JSON file should be modified as :

example_session_template.json

[
  {
    "id": "c1b8441a-ba46-4a83-8727-de6645f521b4",
    "is_active": true,
    "domain_name": "default",
    "group_id": "2de2b969-1d04-48a6-af16-0bc8adb3c831",
    "user_uuid": "f38dea23-50fa-42a0-b5ae-338f5f4693f4",
    "type": "TASK",
    "name": "python_x86",
    "template": {
      "api_version": "6",
      "kind": "task_template",
      "metadata": {
        "name": "cr.backend.ai/multiarch/python",
        "tag": "3.10-ubuntu20.04"
      },
      "spec": {
        "session_type": "interactive",
        "kernel": {
          "image": "cr.backend.ai/multiarch/python:3.10-ubuntu20.04",
          "environ": {},
          "architecture": "x86_64",
          "run": null,
          "git": null
        },
        "scaling_group": "default",
        "mounts": {
        },
        "resources": {
          "cpu": "2",
          "mem": "4g"
        }
      }
    }
  },
  {
    "id": "59062449-4f57-4434-975d-add2a593438c",
    "is_active": true,
    "domain_name": "default",
    "group_id": "2de2b969-1d04-48a6-af16-0bc8adb3c831",
    "user_uuid": "f38dea23-50fa-42a0-b5ae-338f5f4693f4",
    "type": "TASK",
    "name": "python_arch64",
    "template": {
      "api_version": "6",
      "kind": "task_template",
      "metadata": {
        "name": "cr.backend.ai/multiarch/python",
        "tag": "3.10-ubuntu20.04"
      },
      "spec": {
        "session_type": "interactive",
        "kernel": {
          "image": "cr.backend.ai/multiarch/python:3.10-ubuntu20.04",
          "environ": {},
          "architecture": "aarch64",
          "run": null,
          "git": null
        },
        "scaling_group": "default",
        "mounts": {
        },
        "resources": {
          "cpu": "2",
          "mem": "4g"
        }
      }
    }
  }
]

example_session_template.yaml

---
id: c1b8441a-ba46-4a83-8727-de6645f521b4
is_active: true
domain_name: default
group_id: 2de2b969-1d04-48a6-af16-0bc8adb3c831
user_uuid: f38dea23-50fa-42a0-b5ae-338f5f4693f4
type: TASK
name: python_x86
template:
  api_version: '6'
  kind: task_template
  metadata:
    name: cr.backend.ai/multiarch/python
    tag: 3.10-ubuntu20.04
  spec:
    session_type: interactive
    kernel:
      image: cr.backend.ai/multiarch/python:3.10-ubuntu20.04
      environ: {}
      architecture: x86_64
      run: null
      git: null
    scaling_group: default
    mounts: {}
    resources:
      cpu: '2'
      mem: 4g
---
id: 59062449-4f57-4434-975d-add2a593438c
is_active: true
domain_name: default
group_id: 2de2b969-1d04-48a6-af16-0bc8adb3c831
user_uuid: f38dea23-50fa-42a0-b5ae-338f5f4693f4
type: TASK
name: python_arch64
template:
  api_version: '6'
  kind: task_template
  metadata:
    name: cr.backend.ai/multiarch/python
    tag: 3.10-ubuntu20.04
  spec:
    session_type: interactive
    kernel:
      image: cr.backend.ai/multiarch/python:3.10-ubuntu20.04
      environ: {}
      architecture: aarch64
      run: null
      git: null
    scaling_group: default
    mounts: {}
    resources:
      cpu: '2'
      mem: 4g

In src/ai/backend/client/func/session_template.py, I modifed cls() argument to return the log with multiple template_id like this:

Task template 868647ae08f94da38a2cf1a30bddb6e6, 76e7bed57f58487e934f5746dc5fdcb1 created and ready

Minor fixes

revise the incorrect parameter order

ai/backend/manager/utils.py

async def query_userinfo(
    conn: SAConnection,
    requester_uuid: UUID,
    requester_access_key: AccessKey,
    requester_role: UserRole,

before ai/backend/manager/api/session.py

return await _query_userinfo(
            conn,
            request["user"]["uuid"],
            request["user"]["role"],
            request["keypair"]["access_key"],

add the missing argument callback_url in src/ai/backend/client/func/session.py

ai/backend/client/cli/session.py

        with Session() as session:
            try:
                compute_session = session.ComputeSession.create_from_template(
                    template_id,
                    image=image,
                    name=name,
                    type_=type_,
                    starts_at=starts_at,
                    enqueue_only=enqueue_only,
                    max_wait=max_wait,
                    no_reuse=no_reuse,
                    dependencies=depends,
                    callback_url=callback_url,
                    cluster_size=cluster_size,

before ai/backend/client/func/session.py

async def create_from_template(
        cls,
        template_id: str,
        *,
        name: str | Undefined = undefined,
        type_: str | Undefined = undefined,
        starts_at: str = None,
        enqueue_only: bool | Undefined = undefined,
        max_wait: int | Undefined = undefined,
        # missing callback_url
        dependencies: Sequence[str] = None,  # cannot be stored in templates
        no_reuse: bool | Undefined = undefined,

cla-assistant · 2023-07-21T04:28:36Z

All committers have signed the CLA.

cla-assistant · 2023-07-21T04:28:37Z

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

JinMyeong Kim seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

fregataa

Thanks for your commitment!
I left some reviews

fregataa · 2023-07-21T04:38:25Z

changes/1393.feature.md

+
+
+
+
+
+
+


Please remove this blank new lines.
And we write news fragment in one line. Please refer other merged PR's news fragment files.
Fragment files are listed in the release notes and read by users. Please note that!

fregataa · 2023-07-21T05:05:24Z

src/ai/backend/manager/api/session_template.py

    async with root_ctx.db.begin() as conn:
        user_uuid, group_id, _ = await query_userinfo(request, params, conn)
        log.debug("Params: {0}", params)
        try:
            body = json.loads(params["payload"])
        except json.JSONDecodeError:
            try:
-                body = yaml.safe_load(params["payload"])
+                body = yaml.safe_load_all(params["payload"])


fregataa · 2023-07-21T05:11:37Z

src/ai/backend/client/cli/params.py

+    def convert(self, value, param, ctx):
+        try:
+            if isinstance(value, (str, int)) or value == undefined:
+                return value
+        except ValueError as e:
+            self.fail(repr(e), param, ctx)


Suggested change

def convert(self, value, param, ctx):

try:

if isinstance(value, (str, int)) or value == undefined:

return value

except ValueError as e:

self.fail(repr(e), param, ctx)

def __init__(self, type_: type) -> None:

super().__init__()

self.type_ = type_

def convert(self, value: Any, param, ctx):

if isinstance(value, self.type_) or value is undefined:

return value

self.fail(f"{value!r} is not valid `{self.type_}` or `undefined`", param, ctx)

Let OptionalType handle not only str and int types, but also Any given type.
And it is good to pass detail failure messages.
Then, OptionalType can be used like below

# example @click.option("--name", type=OptionalType(str), default=undefined, help="Example option.")

fregataa

👍

fregataa · 2023-07-26T05:48:27Z

it would be nice to embed an example yaml file to test session template!

kimjinmyeong · 2023-07-26T06:21:09Z

@fregataa Okay!

kimjinmyeong · 2023-08-03T02:45:06Z

src/ai/backend/manager/api/session.py

            tx.AliasedKey(["image", "lang"], default=undefined): UndefChecker | t.Null | t.String,
-            tx.AliasedKey(["arch", "architecture"], default=DEFAULT_IMAGE_ARCH)
-            >> "architecture": t.String,
-            tx.AliasedKey(["type", "sessionType"], default="interactive")
-            >> "session_type": tx.Enum(SessionTypes),


When a default value is set in here, the configuration value of the template gets overwritten by this default value.

…click-argument-type

kimjinmyeong · 2023-08-16T08:19:24Z

src/ai/backend/client/cli/session.py

@@ -538,7 +550,8 @@ def _create_from_template_cmd(docs: str = None):
        "-g",
        "--group",
        metavar="GROUP_NAME",
-        default=None,


If a None value is passed, the group value gets overwritten with None, leading to an error.

collected an error log [ERROR] "ai.backend.manager.api.exceptions.InvalidAPIParameters: Missing or invalid API parameters. (Invalid group)" from manager

JinMyeong Kim added 7 commits July 20, 2023 07:21

fix: revise the incorrect parameter order

946b52e

fix: add the missing argument

1dfa3b1

fix: revise to support yaml formatted template

6bf92ab

feat: implement OptionalType class

9fe278e

docs: add 1393.feature.md

0e1d886

docs: write PR summary

741b44b

docs: supplement contents

2fb41f1

kimjinmyeong added type:feature Add new features comp:manager Related to Manager component comp:cli Related to CLI component effort:easy Need to understand only a specific region of codes (good first issue, easy). size:M 30~100 LoC labels Jul 21, 2023

kimjinmyeong requested review from achimnol, kyujin-cho and fregataa July 21, 2023 04:28

kimjinmyeong self-assigned this Jul 21, 2023

github-actions bot added comp:client Related to Client component type:bug Reports about that are not working type:refactor Refactor codes or add tests. urgency:2 With time limit, it should be finished within it; otherwise, resolve it when no other chores. labels Jul 21, 2023

fregataa requested changes Jul 21, 2023

View reviewed changes

JinMyeong Kim added 3 commits July 21, 2023 06:50

refactor: modify to support any types

3b03ed0

docs: split news fregment into feature and fix

27eaf98

refactor: add metavar

8c9ded2

refactor: remove int type exception

8821714

kimjinmyeong requested a review from fregataa July 25, 2023 06:49

fix: remove scaling_group key in overwritten_param_check

3e251bd

fregataa approved these changes Jul 25, 2023

View reviewed changes

Merge branch 'main' into fix/handle-undefined-click-argument-type

9643a14

kimjinmyeong commented Aug 3, 2023

View reviewed changes

JinMyeong Kim added 4 commits August 10, 2023 01:11

Merge branch 'main' into fix/handle-undefined-click-argument-type

ec1a6f0

docs: update news fragments

88af73e

Merge remote-tracking branch 'origin/main' into fix/handle-undefined-…

0e48fed

…click-argument-type

fix: rollback scaling_group check

a7557f9

kimjinmyeong marked this pull request as draft August 16, 2023 03:24

JinMyeong Kim added 2 commits August 16, 2023 16:23

refactor: modify dict access code

491022a

docs: update news fragment

831df6d

kimjinmyeong marked this pull request as ready for review August 16, 2023 08:07

kimjinmyeong commented Aug 16, 2023

View reviewed changes

kyujin-cho added this to the 23.09 milestone Aug 16, 2023

kyujin-cho approved these changes Aug 18, 2023

View reviewed changes

kyujin-cho changed the title ~~feat: Add OptionalType class to handle undefined value at cli argument~~ feat: Add OptionalType click parameter type Aug 18, 2023

kyujin-cho changed the title ~~feat: Add OptionalType click parameter type~~ feat: add OptionalType click parameter type Aug 18, 2023

kyujin-cho added this pull request to the merge queue Aug 18, 2023

Merged via the queue into main with commit f78995c Aug 18, 2023

kyujin-cho deleted the fix/handle-undefined-click-argument-type branch August 18, 2023 04:47

kimjinmyeong mentioned this pull request Aug 19, 2023

feat: Add example-session-templates #1496

Merged

fregataa mentioned this pull request Oct 29, 2023

fix: handle request format changes on session template API #1668

Merged

1 task

rapsealk mentioned this pull request Feb 6, 2024

fix: Reorganize click.option decorators in the session create-from-template command #1890

Merged

7 tasks

This was referenced Dec 10, 2024

Clarify the input handling of session template creation API #3238

Closed

Clarify the input handling of session template CRUD API #3239

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add `OptionalType` click parameter type #1406

feat: add `OptionalType` click parameter type #1406

kimjinmyeong commented Jul 21, 2023 •

edited

Loading

cla-assistant bot commented Jul 21, 2023 •

edited

Loading

cla-assistant bot commented Jul 21, 2023

fregataa left a comment

fregataa Jul 21, 2023

fregataa Jul 21, 2023

fregataa Jul 21, 2023 •

edited

Loading

fregataa left a comment

fregataa commented Jul 26, 2023

kimjinmyeong commented Jul 26, 2023 •

edited

Loading

kimjinmyeong Aug 3, 2023 •

edited

Loading

kimjinmyeong Aug 16, 2023

feat: add OptionalType click parameter type #1406

feat: add OptionalType click parameter type #1406

Conversation

kimjinmyeong commented Jul 21, 2023 • edited Loading

cla-assistant bot commented Jul 21, 2023 • edited Loading

cla-assistant bot commented Jul 21, 2023

fregataa left a comment

Choose a reason for hiding this comment

fregataa Jul 21, 2023

Choose a reason for hiding this comment

fregataa Jul 21, 2023

Choose a reason for hiding this comment

fregataa Jul 21, 2023 • edited Loading

Choose a reason for hiding this comment

fregataa left a comment

Choose a reason for hiding this comment

fregataa commented Jul 26, 2023

kimjinmyeong commented Jul 26, 2023 • edited Loading

kimjinmyeong Aug 3, 2023 • edited Loading

Choose a reason for hiding this comment

kimjinmyeong Aug 16, 2023

Choose a reason for hiding this comment

feat: add `OptionalType` click parameter type #1406

feat: add `OptionalType` click parameter type #1406

kimjinmyeong commented Jul 21, 2023 •

edited

Loading

cla-assistant bot commented Jul 21, 2023 •

edited

Loading

fregataa Jul 21, 2023 •

edited

Loading

kimjinmyeong commented Jul 26, 2023 •

edited

Loading

kimjinmyeong Aug 3, 2023 •

edited

Loading