michael-s-molina commented on code in PR #38200:
URL: https://github.com/apache/superset/pull/38200#discussion_r2846742250


##########
superset-extensions-cli/src/superset_extensions_cli/utils.py:
##########


Review Comment:
   `validate_extension_id`, `validate_extension_name`, `to_kebab_case`, and 
`to_snake_case` are not called from any production code.



##########
superset/extensions/utils.py:
##########


Review Comment:
   This should be
   
   ```
   if is_virtual_path:
         ns_path = (
             f"{source_base_path}/backend/src/"
             f"{ns_name.replace('.', '/')}/__init__.py"
         )
   ```



##########
superset-extensions-cli/src/superset_extensions_cli/utils.py:
##########
@@ -266,39 +276,199 @@ def validate_npm_package_name(name: str) -> None:
         raise ExtensionNameError(f"'{name}' is a reserved npm package name")
 
 
-def generate_extension_names(name: str) -> ExtensionNames:
+def validate_publisher(publisher: str) -> None:
     """
-    Generate all extension name variants from display name input.
+    Validate publisher namespace format.
+
+    Args:
+        publisher: Publisher namespace (e.g., 'my-org')
 
-    Flow: Display Name -> Generate ID -> Derive Technical Names from ID
-    Example: "Hello World" -> "hello-world" -> "helloWorld"/"hello_world" 
(from ID)
+    Raises:
+        ExtensionNameError: If publisher is invalid
+    """
+    if not publisher:
+        raise ExtensionNameError("Publisher cannot be empty")
+
+    if not PUBLISHER_REGEX.match(publisher):
+        raise ExtensionNameError(
+            "Publisher must start with a letter and contain only lowercase 
letters, numbers, and hyphens (e.g., 'my-org')"
+        )
+
+    # Check for leading/trailing hyphens
+    if publisher.startswith("-"):
+        raise ExtensionNameError("Publisher cannot start with hyphens")
+
+    if publisher.endswith("-"):
+        raise ExtensionNameError("Publisher cannot end with hyphens")
+
+    # Check for consecutive hyphens
+    if "--" in publisher:
+        raise ExtensionNameError("Publisher cannot have consecutive hyphens")

Review Comment:
   This is dead code given that line 292 already checks for these with the 
regex.
   
   ```suggestion
   ```



##########
superset-core/src/superset_core/extensions/constants.py:
##########
@@ -0,0 +1,35 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""
+Constants for extension validation and naming.
+"""
+
+# Publisher validation pattern: lowercase letters, numbers, hyphens; must 
start with
+# letter; no consecutive hyphens or trailing hyphens
+PUBLISHER_PATTERN = r"^[a-z]([a-z0-9]+(-[a-z0-9]+)*)?$"
+
+# Technical name validation pattern: lowercase letters, numbers, hyphens; must 
start
+# with letter; no consecutive hyphens or trailing hyphens
+TECHNICAL_NAME_PATTERN = r"^[a-z]([a-z0-9]+(-[a-z0-9]+)*)?$"
+
+# Display name validation pattern: must start with letter, can contain letters,
+# numbers, spaces, hyphens, underscores, dots, exclamation marks
+DISPLAY_NAME_PATTERN = r"^[a-zA-Z][a-zA-Z0-9\s\-_\.!]*$"

Review Comment:
   Should we allow exclamation marks?



##########
superset-core/src/superset_core/extensions/constants.py:
##########
@@ -0,0 +1,35 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""
+Constants for extension validation and naming.
+"""
+
+# Publisher validation pattern: lowercase letters, numbers, hyphens; must 
start with
+# letter; no consecutive hyphens or trailing hyphens
+PUBLISHER_PATTERN = r"^[a-z]([a-z0-9]+(-[a-z0-9]+)*)?$"

Review Comment:
   ```suggestion
   PUBLISHER_PATTERN = r"^[a-z][a-z0-9]*(-[a-z0-9]+)*$"
   ```



##########
superset-core/src/superset_core/extensions/constants.py:
##########
@@ -0,0 +1,35 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""
+Constants for extension validation and naming.
+"""
+
+# Publisher validation pattern: lowercase letters, numbers, hyphens; must 
start with
+# letter; no consecutive hyphens or trailing hyphens
+PUBLISHER_PATTERN = r"^[a-z]([a-z0-9]+(-[a-z0-9]+)*)?$"
+
+# Technical name validation pattern: lowercase letters, numbers, hyphens; must 
start
+# with letter; no consecutive hyphens or trailing hyphens
+TECHNICAL_NAME_PATTERN = r"^[a-z]([a-z0-9]+(-[a-z0-9]+)*)?$"

Review Comment:
   ```suggestion
   TECHNICAL_NAME_PATTERN = r"^[a-z][a-z0-9]*(-[a-z0-9]+)*$"
   ```



-- 
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