andrewmusselman commented on code in PR #309:
URL: 
https://github.com/apache/tooling-trusted-releases/pull/309#discussion_r2562284806


##########
atr/shared/distribution.py:
##########
@@ -17,74 +17,125 @@
 
 from __future__ import annotations
 
-import dataclasses
-import json
-from typing import Literal
+import enum
+from typing import Any, Literal, cast
 
-import quart
+import pydantic
 
 import atr.db as db
-import atr.forms as forms
+import atr.form as form
 import atr.get as get
 import atr.htm as htm
 import atr.models.distribution as distribution
 import atr.models.sql as sql
-import atr.storage as storage
-import atr.template as template
 import atr.util as util
 
 type Phase = Literal["COMPOSE", "VOTE", "FINISH"]
 
 
-class DeleteForm(forms.Typed):
-    release_name = forms.hidden()
-    platform = forms.hidden()
-    owner_namespace = forms.hidden()
-    package = forms.hidden()
-    version = forms.hidden()
-    submit = forms.submit("Delete")
-
-
-class DistributeForm(forms.Typed):
-    platform = forms.select("Platform", choices=sql.DistributionPlatform)
-    owner_namespace = forms.optional(
+class DistributionPlatformWrapper(enum.Enum):
+    """Wrapper enum for distribution platforms."""
+
+    ARTIFACT_HUB = "Artifact Hub"
+    DOCKER_HUB = "Docker Hub"
+    MAVEN = "Maven Central"
+    NPM = "npm"
+    NPM_SCOPED = "npm (scoped)"
+    PYPI = "PyPI"
+
+
+def wrapper_to_sql(wrapper: DistributionPlatformWrapper) -> 
sql.DistributionPlatform:
+    """Convert wrapper enum to SQL enum."""
+    match wrapper:
+        case DistributionPlatformWrapper.ARTIFACT_HUB:
+            return sql.DistributionPlatform.ARTIFACT_HUB
+        case DistributionPlatformWrapper.DOCKER_HUB:
+            return sql.DistributionPlatform.DOCKER_HUB
+        case DistributionPlatformWrapper.MAVEN:
+            return sql.DistributionPlatform.MAVEN
+        case DistributionPlatformWrapper.NPM:
+            return sql.DistributionPlatform.NPM
+        case DistributionPlatformWrapper.NPM_SCOPED:
+            return sql.DistributionPlatform.NPM_SCOPED
+        case DistributionPlatformWrapper.PYPI:
+            return sql.DistributionPlatform.PYPI
+
+
+def sql_to_wrapper(platform: sql.DistributionPlatform) -> 
DistributionPlatformWrapper:
+    """Convert SQL enum to wrapper enum."""
+    match platform:
+        case sql.DistributionPlatform.ARTIFACT_HUB:
+            return DistributionPlatformWrapper.ARTIFACT_HUB
+        case sql.DistributionPlatform.DOCKER_HUB:
+            return DistributionPlatformWrapper.DOCKER_HUB
+        case sql.DistributionPlatform.MAVEN:
+            return DistributionPlatformWrapper.MAVEN
+        case sql.DistributionPlatform.NPM:
+            return DistributionPlatformWrapper.NPM
+        case sql.DistributionPlatform.NPM_SCOPED:
+            return DistributionPlatformWrapper.NPM_SCOPED
+        case sql.DistributionPlatform.PYPI:
+            return DistributionPlatformWrapper.PYPI
+
+
+class DeleteForm(form.Form):
+    release_name: str = form.label("Release name", widget=form.Widget.HIDDEN)
+    platform: str = form.label("Platform", widget=form.Widget.HIDDEN)
+    owner_namespace: str = form.label("Owner namespace", 
widget=form.Widget.HIDDEN)
+    package: str = form.label("Package", widget=form.Widget.HIDDEN)
+    version: str = form.label("Version", widget=form.Widget.HIDDEN)
+
+
+class DistributeForm(form.Form):
+    platform: form.Enum[DistributionPlatformWrapper] = 
form.label(description="Platform")
+    owner_namespace: str = form.label(
         "Owner or Namespace",
-        placeholder="E.g. com.example or scope or library",
-        description="Who owns or names the package (Maven groupId, npm @scope, 
"
-        "Docker namespace, GitHub owner, ArtifactHub repo). Leave blank if not 
used.",
+        """Who owns or names the package (Maven groupId, npm @scope, Docker 
namespace,
+            GitHub owner, ArtifactHub repo). Leave blank if not used.""",
     )
-    package = forms.string("Package", placeholder="E.g. artifactId or 
package-name")
-    version = forms.string("Version", placeholder="E.g. 1.2.3, without a 
leading v")
-    details = forms.checkbox("Include details", description="Include the 
details of the distribution in the response")
-    submit = forms.submit("Record distribution")
-
-    async def validate(self, extra_validators: dict | None = None) -> bool:
-        if not await super().validate(extra_validators):
-            return False
-        if not self.platform.data:
-            return False
-        default_owner_namespace = 
self.platform.data.value.default_owner_namespace
-        requires_owner_namespace = 
self.platform.data.value.requires_owner_namespace
-        owner_namespace = self.owner_namespace.data
-        # TODO: We should disable the owner_namespace field if it's not 
required
-        # But that would be a lot of complexity
-        # And this validation, which we need to keep, is complex enough
-        if default_owner_namespace and (not owner_namespace):
-            self.owner_namespace.data = default_owner_namespace
-        if requires_owner_namespace and (not owner_namespace):
-            msg = f'Platform "{self.platform.data.name}" requires an owner or 
namespace.'
-            return forms.error(self.owner_namespace, msg)
-        if (not requires_owner_namespace) and (not default_owner_namespace) 
and owner_namespace:
-            msg = f'Platform "{self.platform.data.name}" does not require an 
owner or namespace.'
-            return forms.error(self.owner_namespace, msg)
-        return True
-
-
[email protected]
-class FormProjectVersion:
-    form: DistributeForm
-    project: str
-    version: str
+    package: str = form.label("Package", widget=form.Widget.TEXT)
+    version: str = form.label("Version", widget=form.Widget.TEXT)
+    details: form.Bool = form.label(
+        "Include details",
+        "Include the details of the distribution in the response",
+    )
+
+    @pydantic.field_validator("platform", mode="before")
+    @classmethod
+    def validate_platform(cls, value: Any) -> DistributionPlatformWrapper:
+        if isinstance(value, DistributionPlatformWrapper):
+            return value
+        elif isinstance(value, str):
+            for member in DistributionPlatformWrapper:
+                if member.value == value:
+                    return member
+            raise ValueError(f"Invalid platform: {value}")
+        raise ValueError(f"Platform must be a string or DistributionPlatform, 
got {type(value)}")
+
+    @pydantic.model_validator(mode="after")
+    def validate_owner_namespace(self) -> DistributeForm:
+        if not self.platform:
+            raise ValueError("Platform is required")
+
+        # Help the type checker understand the actual runtime type
+        platform = cast("DistributionPlatformWrapper", self.platform)

Review Comment:
   Yeah, removing it along with other fixes.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to