codeant-ai-for-open-source[bot] commented on code in PR #38448: URL: https://github.com/apache/superset/pull/38448#discussion_r2891745518
########## superset-core/src/superset_core/tasks/daos.py: ########## @@ -0,0 +1,76 @@ +# 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. + +""" +Task Data Access Object API for superset-core. + +Provides task-related DAO classes that will be replaced by host implementations +during initialization. + +Usage: + from superset_core.tasks.daos import TaskDAO +""" + +from abc import abstractmethod + +from superset_core.common.daos import BaseDAO +from superset_core.tasks.models import Task + + +class TaskDAO(BaseDAO[Task]): + """ + Abstract Task DAO interface. + + Host implementations will replace this class during initialization + with a concrete implementation providing actual functionality. + """ + + # Class variables that will be set by host implementation + model_cls = None + base_filter = None + id_column_name = "id" + uuid_column_name = "uuid" + + @classmethod + @abstractmethod + def find_by_task_key( + cls, + task_type: str, + task_key: str, + scope: str = "private", + user_id: int | None = None, + ) -> Task | None: + """ + Find active task by type, key, scope, and user. + + Uses dedup_key internally for efficient querying with a unique index. + Only returns tasks that are active (pending or in progress). + + Uniqueness logic by scope: + - private: scope + task_type + task_key + user_id + - shared/system: scope + task_type + task_key (user-agnostic) + + :param task_type: Task type to filter by + :param task_key: Task identifier for deduplication + :param scope: Task scope (private/shared/system) + :param user_id: User ID (required for private tasks) + :returns: Task instance or None if not found or not active + """ + ... Review Comment: **Suggestion:** The abstract DAO method currently returns `Ellipsis` instead of failing fast, so if it is ever invoked before being replaced by a host implementation it will silently return an invalid value rather than raising an error, breaking any caller that expects a `Task` or `None`. [logic error] <details> <summary><b>Severity Level:</b> Major ⚠️</summary> ```mdx - ⚠️ TaskDAO callers may receive Ellipsis instead of Task/None. - ⚠️ Misconfigured hosts using default TaskDAO fail non-obviously. ``` </details> ```suggestion def find_by_task_key( cls, task_type: str, task_key: str, scope: str = "private", user_id: int | None = None, ) -> Task | None: """ Find active task by type, key, scope, and user. Uses dedup_key internally for efficient querying with a unique index. Only returns tasks that are active (pending or in progress). Uniqueness logic by scope: - private: scope + task_type + task_key + user_id - shared/system: scope + task_type + task_key (user-agnostic) :param task_type: Task type to filter by :param task_key: Task identifier for deduplication :param scope: Task scope (private/shared/system) :param user_id: User ID (required for private tasks) :returns: Task instance or None if not found or not active """ raise NotImplementedError("Method will be replaced during initialization") ``` <details> <summary><b>Steps of Reproduction ✅ </b></summary> ```mdx 1. Import the abstract DAO interface `TaskDAO` from `superset-core/src/superset_core/tasks/daos.py` as documented in the module usage comment (`from superset_core.tasks.daos import TaskDAO`). 2. Call the class method `TaskDAO.find_by_task_key(...)` before any host application has replaced `TaskDAO` with a concrete implementation (the method definition is at `superset_core/tasks/daos.py:50-73`). 3. Python executes the method body, which consists solely of `...` (Ellipsis) at `superset_core/tasks/daos.py:73`, and returns the `Ellipsis` singleton instead of a `Task` instance or `None`. 4. Any caller that expects the annotated return type `Task | None` (per the method signature and docstring at `superset_core/tasks/daos.py:50-72`) will instead receive an `Ellipsis` object, leading to incorrect control flow or type errors when the value is used (e.g., in truthiness checks or attribute access). This stems from the placeholder implementation rather than an explicit failure. ``` </details> <details> <summary><b>Prompt for AI Agent 🤖 </b></summary> ```mdx This is a comment left during a code review. **Path:** superset-core/src/superset_core/tasks/daos.py **Line:** 50:73 **Comment:** *Logic Error: The abstract DAO method currently returns `Ellipsis` instead of failing fast, so if it is ever invoked before being replaced by a host implementation it will silently return an invalid value rather than raising an error, breaking any caller that expects a `Task` or `None`. Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise. ``` </details> <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38448&comment_hash=0c017d0b35fd799bc44503cfac304dd16a4987417af5eedb8a4b0da36b4ddcc5&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38448&comment_hash=0c017d0b35fd799bc44503cfac304dd16a4987417af5eedb8a4b0da36b4ddcc5&reaction=dislike'>👎</a> -- 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]
