feat: Make skill_id a required parameter in skills.create

agentplatform/_genai/skills.py

@@ -55,9 +55,6 @@ def _CreateSkillConfig_to_vertex(
             getv(from_object, ["zipped_filesystem"]),
         )
 
-    if getv(from_object, ["skill_id"]) is not None:
-        setv(parent_object, ["_query", "skillId"], getv(from_object, ["skill_id"]))
-
     return to_object
 
 
@@ -75,6 +72,9 @@ def _CreateSkillRequestParameters_to_vertex(
     if getv(from_object, ["config"]) is not None:
         _CreateSkillConfig_to_vertex(getv(from_object, ["config"]), to_object)
 
+    if getv(from_object, ["skill_id"]) is not None:
+        setv(to_object, ["_query", "skillId"], getv(from_object, ["skill_id"]))
+
     return to_object
 
 
@@ -367,6 +367,7 @@ def _create(
         display_name: str,
         description: str,
         config: Optional[types.CreateSkillConfigOrDict] = None,
+        skill_id: str,
     ) -> types.SkillOperation:
         """
         Creates a new Skill.
@@ -376,6 +377,7 @@ def _create(
             display_name=display_name,
             description=description,
             config=config,
+            skill_id=skill_id,
         )
 
         request_url_dict: Optional[dict[str, str]]
@@ -712,13 +714,17 @@ def _get_skill_operation(
     def create(
         self,
         *,
+        skill_id: str,
         display_name: str,
         description: str,
         config: Optional[types.CreateSkillConfigOrDict] = None,
     ) -> Union[types.Skill, types.SkillOperation]:
         """Creates a new Skill.
 
         Args:
+            skill_id (str):
+                Required. The ID to use for the Skill, which will become the final
+                component of the Skill's resource name.
             display_name (str):
                 Required. The display name of the Skill.
             description (str):
@@ -775,6 +781,7 @@ def create(
         config.zipped_filesystem = zipped_filesystem_payload
 
         operation = self._create(
+            skill_id=skill_id,
             display_name=display_name,
             description=description,
             config=config,
@@ -1110,6 +1117,7 @@ async def _create(
         display_name: str,
         description: str,
         config: Optional[types.CreateSkillConfigOrDict] = None,
+        skill_id: str,
     ) -> types.SkillOperation:
         """
         Creates a new Skill.
@@ -1119,6 +1127,7 @@ async def _create(
             display_name=display_name,
             description=description,
             config=config,
+            skill_id=skill_id,
         )
 
         request_url_dict: Optional[dict[str, str]]
@@ -1465,13 +1474,17 @@ async def _get_skill_operation(
     async def create(
         self,
         *,
+        skill_id: str,
         display_name: str,
         description: str,
         config: Optional[types.CreateSkillConfigOrDict] = None,
     ) -> Union[types.Skill, types.SkillOperation]:
         """Creates a new Skill asynchronously.
 
         Args:
+            skill_id (str):
+                Required. The ID to use for the Skill, which will become the final
+                component of the Skill's resource name.
             display_name (str):
                 Required. The display name of the Skill.
             description (str):
@@ -1529,6 +1542,7 @@ async def create(
         config.zipped_filesystem = zipped_filesystem_payload
 
         operation = await self._create(
+            skill_id=skill_id,
             display_name=display_name,
             description=description,
             config=config,

agentplatform/_genai/types/common.py

@@ -18262,12 +18262,6 @@ class CreateSkillConfig(_common.BaseModel):
     zipped_filesystem: Optional[Any] = Field(
         default=None, description="""Optional. The zipped filesystem of the Skill."""
     )
-    skill_id: Optional[str] = Field(
-        default=None,
-        description="""Optional. The ID to use for the Skill, which will become the final
-      component of the Skill's resource name.
-      """,
-    )
 
 
 class CreateSkillConfigDict(TypedDict, total=False):
@@ -18287,11 +18281,6 @@ class CreateSkillConfigDict(TypedDict, total=False):
     zipped_filesystem: Optional[Any]
     """Optional. The zipped filesystem of the Skill."""
 
-    skill_id: Optional[str]
-    """Optional. The ID to use for the Skill, which will become the final
-      component of the Skill's resource name.
-      """
-
 
 CreateSkillConfigOrDict = Union[CreateSkillConfig, CreateSkillConfigDict]
 
@@ -18306,6 +18295,12 @@ class _CreateSkillRequestParameters(_common.BaseModel):
         default=None, description="""Required. The description of the Skill."""
     )
     config: Optional[CreateSkillConfig] = Field(default=None, description="""""")
+    skill_id: Optional[str] = Field(
+        default=None,
+        description="""Required. The ID to use for the Skill, which will become the final
+      component of the Skill's resource name.
+      """,
+    )
 
 
 class _CreateSkillRequestParametersDict(TypedDict, total=False):
@@ -18320,6 +18315,11 @@ class _CreateSkillRequestParametersDict(TypedDict, total=False):
     config: Optional[CreateSkillConfigDict]
     """"""
 
+    skill_id: Optional[str]
+    """Required. The ID to use for the Skill, which will become the final
+      component of the Skill's resource name.
+      """
+
 
 _CreateSkillRequestParametersOrDict = Union[
     _CreateSkillRequestParameters, _CreateSkillRequestParametersDict

tests/unit/agentplatform/genai/replays/test_skills_create.py

@@ -28,47 +28,49 @@
 
 
 def test_create_skill(client, tmp_path):
-    client._api_client._http_options.base_url = (
-        "https://us-central1-aiplatform.googleapis.com"
-    )
+  client._api_client._http_options.base_url = (
+      "https://us-central1-aiplatform.googleapis.com"
+  )
 
-    # Create a dummy skill structure (SKILL.md is required by the spec)
-    with open(tmp_path / "SKILL.md", "w") as f:
-        f.write("# My Replay Skill\nThis is a test skill for replay tests.")
+  # Create a dummy skill structure (SKILL.md is required by the spec)
+  with open(tmp_path / "SKILL.md", "w") as f:
+    f.write("# My Replay Skill\nThis is a test skill for replay tests.")
 
-    skill = client.skills.create(
-        display_name="My Replay Skill",
-        description="My Replay Skill Description",
-        config=types.CreateSkillConfig(
-            local_path=str(tmp_path), wait_for_completion=True
-        ),
-    )
+  skill = client.skills.create(
+      skill_id="my-replay-skill-v2",
+      display_name="My Replay Skill",
+      description="My Replay Skill Description",
+      config=types.CreateSkillConfig(
+          local_path=str(tmp_path), wait_for_completion=True
+      ),
+  )
 
-    assert skill.name is not None
-    assert skill.display_name == "My Replay Skill"
-    assert skill.description == "My Replay Skill Description"
+  assert skill.name is not None
+  assert skill.display_name == "My Replay Skill"
+  assert skill.description == "My Replay Skill Description"
 
 
 def test_create_skill_with_prezipped_bytes(client):
-    """Tests the creation of a skill with pre-zipped bytes."""
-    client._api_client._http_options.base_url = (
-        "https://us-central1-aiplatform.googleapis.com"
-    )
+  """Tests the creation of a skill with pre-zipped bytes."""
+  client._api_client._http_options.base_url = (
+      "https://us-central1-aiplatform.googleapis.com"
+  )
 
-    zip_buffer = io.BytesIO()
-    zinfo = zipfile.ZipInfo("SKILL.md", date_time=(1980, 1, 1, 0, 0, 0))
-    with zipfile.ZipFile(zip_buffer, "w") as zip_file:
-        zip_file.writestr(zinfo, "# My Zipped Replay Skill\nThis is a test.")
-    zipped_bytes = zip_buffer.getvalue()
+  zip_buffer = io.BytesIO()
+  zinfo = zipfile.ZipInfo("SKILL.md", date_time=(1980, 1, 1, 0, 0, 0))
+  with zipfile.ZipFile(zip_buffer, "w") as zip_file:
+    zip_file.writestr(zinfo, "# My Zipped Replay Skill\nThis is a test.")
+  zipped_bytes = zip_buffer.getvalue()
 
-    skill = client.skills.create(
-        display_name="My Zipped Replay Skill",
-        description="My Zipped Replay Skill Description",
-        config=types.CreateSkillConfig(
-            zipped_filesystem=zipped_bytes, wait_for_completion=True
-        ),
-    )
+  skill = client.skills.create(
+      skill_id="my-zipped-replay-skill-v2",
+      display_name="My Zipped Replay Skill",
+      description="My Zipped Replay Skill Description",
+      config=types.CreateSkillConfig(
+          zipped_filesystem=zipped_bytes, wait_for_completion=True
+      ),
+  )
 
-    assert skill.name is not None
-    assert skill.display_name == "My Zipped Replay Skill"
-    assert skill.description == "My Zipped Replay Skill Description"
+  assert skill.name is not None
+  assert skill.display_name == "My Zipped Replay Skill"
+  assert skill.description == "My Zipped Replay Skill Description"

tests/unit/agentplatform/genai/replays/test_skills_delete.py

@@ -13,28 +13,29 @@
 
 def test_delete_skill(client, tmp_path):
 
-    # 1. Create a fresh unique skill first
-    with open(tmp_path / "SKILL.md", "w") as f:
-        f.write("# Test Skill\nTo be deleted.")
-
-    created_skill = client.skills.create(
-        display_name="To Be Deleted Skill",
-        description="Skill to be deleted",
-        config=types.CreateSkillConfig(
-            local_path=str(tmp_path), wait_for_completion=True
-        ),
-    )
-
-    assert created_skill.name is not None
-
-    # 2. Delete the skill and wait for LRO completion
-    client.skills.delete(
+  # 1. Create a fresh unique skill first
+  with open(tmp_path / "SKILL.md", "w") as f:
+    f.write("# Test Skill\nTo be deleted.")
+
+  created_skill = client.skills.create(
+      skill_id="my-skill-to-delete",
+      display_name="To Be Deleted Skill",
+      description="Skill to be deleted",
+      config=types.CreateSkillConfig(
+          local_path=str(tmp_path), wait_for_completion=True
+      ),
+  )
+
+  assert created_skill.name is not None
+
+  # 2. Delete the skill and wait for LRO completion
+  client.skills.delete(
         name=created_skill.name,
         config=types.DeleteSkillConfig(wait_for_completion=True),
     )
 
-    # 3. Verify the skill is successfully deleted (Getting it should raise NotFound)
-    with pytest.raises(errors.ClientError) as exc_info:
-        client.skills.get(name=created_skill.name)
+  # 3. Verify the skill is successfully deleted (Getting it should raise NotFound)
+  with pytest.raises(errors.ClientError) as exc_info:
+    client.skills.get(name=created_skill.name)
 
-    assert exc_info.value.code == 404
+  assert exc_info.value.code == 404

tests/unit/agentplatform/genai/replays/test_skills_revisions_get.py

@@ -19,45 +19,46 @@
 
 
 def test_get_skill_revision(client, tmp_path):
-    # Target the autopush sandbox endpoint for the Skill Registry API
-    client._api_client._http_options.base_url = (
+  # Target the autopush sandbox endpoint for the Skill Registry API
+  client._api_client._http_options.base_url = (
         "https://us-central1-autopush-aiplatform.sandbox.googleapis.com"
     )
 
-    # 1. Create a fresh unique skill
-    with open(tmp_path / "SKILL.md", "w") as f:
-        f.write("# Replay Revision Test Skill\nThis is a test skill.")
+  # 1. Create a fresh unique skill
+  with open(tmp_path / "SKILL.md", "w") as f:
+    f.write("# Replay Revision Test Skill\nThis is a test skill.")
 
-    created_skill = client.skills.create(
-        display_name="Replay Revision Test Skill",
-        description="A temporary skill to test revisions E2E",
-        config=types.CreateSkillConfig(
-            local_path=str(tmp_path), wait_for_completion=True
-        ),
-    )
+  created_skill = client.skills.create(
+      skill_id="my-skill-to-get-revision",
+      display_name="Replay Revision Test Skill",
+      description="A temporary skill to test revisions E2E",
+      config=types.CreateSkillConfig(
+          local_path=str(tmp_path), wait_for_completion=True
+      ),
+  )
 
-    try:
-        assert created_skill.name is not None
+  try:
+    assert created_skill.name is not None
 
-        # 2. List revisions to dynamically discover the revision ID
-        revisions_response = client.skills.revisions.list(name=created_skill.name)
-        revisions_list = revisions_response.skill_revisions
+    # 2. List revisions to dynamically discover the revision ID
+    revisions_response = client.skills.revisions.list(name=created_skill.name)
+    revisions_list = revisions_response.skill_revisions
 
-        assert len(revisions_list) > 0
-        first_revision = revisions_list[0]
-        assert isinstance(first_revision, types.SkillRevision)
-        assert first_revision.name is not None
+    assert len(revisions_list) > 0
+    first_revision = revisions_list[0]
+    assert isinstance(first_revision, types.SkillRevision)
+    assert first_revision.name is not None
 
-        # 3. Explicitly GET the revision by its resource name
-        revision = client.skills.revisions.get(name=first_revision.name)
+    # 3. Explicitly GET the revision by its resource name
+    revision = client.skills.revisions.get(name=first_revision.name)
 
-        assert isinstance(revision, types.SkillRevision)
-        assert revision.name == first_revision.name
-        assert revision.state == types.SkillState.ACTIVE
+    assert isinstance(revision, types.SkillRevision)
+    assert revision.name == first_revision.name
+    assert revision.state == types.SkillState.ACTIVE
 
-    finally:
-        # 4. Clean up the temporary skill
-        client.skills.delete(
+  finally:
+    # 4. Clean up the temporary skill
+    client.skills.delete(
             name=created_skill.name,
             config=types.DeleteSkillConfig(wait_for_completion=True),
         )