langgenius/dify
1
0
Fork 0
mirror of https://github.com/langgenius/dify.git synced 2026-07-20 19:50:06 -04:00
Code Issues 775 Packages Projects Releases 10 Wiki Activity

it's not recommend to use abstract method in normal class #22247

New Issue
Open
opened 2026-02-21 20:16:18 -05:00 by yindo · 3 comments
yindo commented 2026-02-21 20:16:18 -05:00
Owner
Copy Link
Copy Source

Originally created by @asukaminato0721 on GitHub (Feb 18, 2026).

Self Checks

  • I have read the Contributing Guide and Language Policy.
  • This is only for bug report, if you would like to ask a question, please head to Discussions.
  • I have searched for existing issues search for existing issues, including closed ones.
  • I confirm that I am using English to submit this report, otherwise it will be closed.
  • 【中文用户 & Non English User】请使用英语提交，否则会被关闭 ：）
  • Please do not modify this template :) and fill in all the required fields.

Dify version

master

Cloud or Self Hosted

Self Hosted (Source)

Steps to reproduce

class AppQueueManager:

dont inherit ABC but have

    @abstractmethod
    def _publish(self, event: AppQueueEvent, pub_from: PublishFrom):
        """
        Publish event to queue
        :param event:
        :param pub_from:
        :return:
        """
        raise NotImplementedError

and overwrite in child class

    def _publish(self, event: AppQueueEvent, pub_from: PublishFrom):
        """
        Publish event to queue
        :param event:
        :param pub_from:
        :return:
        """
        message = WorkflowQueueMessage(task_id=self._task_id, app_mode=self._app_mode, event=event)

        self._q.put(message)

        if isinstance(
            event,
            QueueStopEvent
            | QueueErrorEvent
            | QueueMessageEndEvent
            | QueueWorkflowSucceededEvent
            | QueueWorkflowFailedEvent
            | QueueWorkflowPartialSuccessEvent,
        ):
            self.stop_listen()

        if pub_from == PublishFrom.APPLICATION_MANAGER and self._is_stopped():
            raise GenerateTaskStoppedError()

✔️ Expected Behavior

https://stackoverflow.com/questions/49051638/python-abstract-method-in-normal-class

❌ Actual Behavior

No response

Originally created by @asukaminato0721 on GitHub (Feb 18, 2026). ### Self Checks - [x] I have read the [Contributing Guide](https://github.com/langgenius/dify/blob/main/CONTRIBUTING.md) and [Language Policy](https://github.com/langgenius/dify/issues/1542). - [x] This is only for bug report, if you would like to ask a question, please head to [Discussions](https://github.com/langgenius/dify/discussions/categories/general). - [x] I have searched for existing issues [search for existing issues](https://github.com/langgenius/dify/issues), including closed ones. - [x] I confirm that I am using English to submit this report, otherwise it will be closed. - [x] 【中文用户 & Non English User】请使用英语提交，否则会被关闭 ：） - [x] Please do not modify this template :) and fill in all the required fields. ### Dify version master ### Cloud or Self Hosted Self Hosted (Source) ### Steps to reproduce ``` class AppQueueManager: ``` dont inherit ABC but have ``` @abstractmethod def _publish(self, event: AppQueueEvent, pub_from: PublishFrom): """ Publish event to queue :param event: :param pub_from: :return: """ raise NotImplementedError ``` and overwrite in child class ```py def _publish(self, event: AppQueueEvent, pub_from: PublishFrom): """ Publish event to queue :param event: :param pub_from: :return: """ message = WorkflowQueueMessage(task_id=self._task_id, app_mode=self._app_mode, event=event) self._q.put(message) if isinstance( event, QueueStopEvent | QueueErrorEvent | QueueMessageEndEvent | QueueWorkflowSucceededEvent | QueueWorkflowFailedEvent | QueueWorkflowPartialSuccessEvent, ): self.stop_listen() if pub_from == PublishFrom.APPLICATION_MANAGER and self._is_stopped(): raise GenerateTaskStoppedError() ``` ### ✔️ Expected Behavior https://stackoverflow.com/questions/49051638/python-abstract-method-in-normal-class ### ❌ Actual Behavior _No response_
yindo added the good first issue🙏 help wanted labels 2026-02-21 20:16:18 -05:00
yindo commented 2026-02-21 20:16:18 -05:00
Author
Owner
Copy Link
Copy Source

@asukaminato0721 commented on GitHub (Feb 18, 2026):

https://github.com/astral-sh/ruff/pull/14688

ruff haven't merge this yet.

@asukaminato0721 commented on GitHub (Feb 18, 2026): https://github.com/astral-sh/ruff/pull/14688 ruff haven't merge this yet.
yindo commented 2026-02-21 20:16:18 -05:00
Author
Owner
Copy Link
Copy Source

@asukaminato0721 commented on GitHub (Feb 18, 2026):

@dosu what is your suggestion?

@asukaminato0721 commented on GitHub (Feb 18, 2026): @dosu what is your suggestion?
yindo commented 2026-02-21 20:16:18 -05:00
Author
Owner
Copy Link
Copy Source

@dosubot[bot] commented on GitHub (Feb 18, 2026):

You're correct that this is an anti-pattern. My suggestion would be to fix it by making AppQueueManager inherit from ABC:

from abc import ABC, abstractmethod

class AppQueueManager(ABC):
    # ... rest of the class

This is a straightforward fix because:

  1. All three subclasses (WorkflowAppQueueManager, MessageBasedAppQueueManager, PipelineQueueManager) already properly implement _publish
  2. AppQueueManager shouldn't be instantiated directly anyway—the @abstractmethod + raise NotImplementedError pattern indicates the intent was always for it to be abstract
  3. Adding ABC inheritance would just add Python's built-in enforcement for what's already the expected behavior

If you'd like to submit a PR for this, it should be a minimal change—just add ABC to the class definition and update the import.

To reply, just mention @dosu.

How did I do? Good | Irrelevant | Incorrect | Verbose | Hallucination | Report 🐛 | Other

Chat with Dosu Open in Cursor Join Discord Share on X

@dosubot[bot] commented on GitHub (Feb 18, 2026): <!-- Answer --> You're correct that this is an anti-pattern. My suggestion would be to fix it by making `AppQueueManager` inherit from `ABC`: ```python from abc import ABC, abstractmethod class AppQueueManager(ABC): # ... rest of the class ``` This is a straightforward fix because: 1. All three subclasses ([WorkflowAppQueueManager](https://github.com/langgenius/dify/blob/41a4a57d2e9e7d41fee5658f503bd350e4786c82/api/core/app/apps/workflow/app_queue_manager.py#L22-L45), [MessageBasedAppQueueManager](https://github.com/langgenius/dify/blob/41a4a57d2e9e7d41fee5658f503bd350e4786c82/api/core/app/apps/message_based_app_queue_manager.py#L24-L47), [PipelineQueueManager](https://github.com/langgenius/dify/blob/41a4a57d2e9e7d41fee5658f503bd350e4786c82/api/core/app/apps/pipeline/pipeline_queue_manager.py#L22-L45)) already properly implement `_publish` 2. `AppQueueManager` shouldn't be instantiated directly anyway—the `@abstractmethod` + `raise NotImplementedError` pattern indicates the intent was always for it to be abstract 3. Adding `ABC` inheritance would just add Python's built-in enforcement for what's already the expected behavior If you'd like to submit a PR for this, it should be a minimal change—just add `ABC` to the [class definition](https://github.com/langgenius/dify/blob/41a4a57d2e9e7d41fee5658f503bd350e4786c82/api/core/app/apps/base_app_queue_manager.py#L34) and update the import. <!-- Dosu Comment Footer --> *To reply, just mention [@dosu](https://go.dosu.dev/dosubot).* --- <sup>How did I do? [Good](https://app.dosu.dev/response-feedback/9b4b37a1-01da-44cf-8643-e3ffcad33b71?feedback_type=great_response) | [Irrelevant](https://app.dosu.dev/response-feedback/9b4b37a1-01da-44cf-8643-e3ffcad33b71?feedback_type=irrelevant_answer) | [Incorrect](https://app.dosu.dev/response-feedback/9b4b37a1-01da-44cf-8643-e3ffcad33b71?feedback_type=incorrect_sources) | [Verbose](https://app.dosu.dev/response-feedback/9b4b37a1-01da-44cf-8643-e3ffcad33b71?feedback_type=too_verbose) | [Hallucination](https://app.dosu.dev/response-feedback/9b4b37a1-01da-44cf-8643-e3ffcad33b71?feedback_type=hallucination) | [Report 🐛](https://app.dosu.dev/response-feedback/9b4b37a1-01da-44cf-8643-e3ffcad33b71?feedback_type=bug_report) | [Other](https://app.dosu.dev/response-feedback/9b4b37a1-01da-44cf-8643-e3ffcad33b71?feedback_type=other)</sup> [![Chat with Dosu](https://dosu.dev/dosu-chat-badge.svg)](https://app.dosu.dev/6088a4f4-073a-43f3-b3d4-4f06fb0c05a0/ask?utm_source=github)&nbsp;[![Open in Cursor](https://dosu.dev/dosu-in-cursor.svg)](https://cursor.com/link/prompt?text=You%27re%20correct%20that%20this%20is%20an%20anti-pattern.%20My%20suggestion%20would%20be%20to%20fix%20it%20by%20making%20%60AppQueueManager%60%20inherit%20from%20%60ABC%60%3A%0A%0A%60%60%60python%0Afrom%20abc%20import%20ABC%2C%20abstractmethod%0A%0Aclass%20AppQueueManager%28ABC%29%3A%0A%20%20%20%20%23%20...%20rest%20of%20the%20class%0A%60%60%60%0A%0AThis%20is%20a%20straightforward%20fix%20because%3A%0A1.%20All%20three%20subclasses%20%28%5BWorkflowAppQueueManager%5D%28https%3A//github.com/langgenius/dify/blob/41a4a57d2e9e7d41fee5658f503bd350e4786c82/api/core/app/apps/workflow/app_queue_manager.py%23L22-L45%29%2C%20%5BMessageBasedAppQueueManager%5D%28https%3A//github.com/langgenius/dify/blob/41a4a57d2e9e7d41fee5658f503bd350e4786c82/api/core/app/apps/message_based_app_queue_manager.py%23L24-L47%29%2C%20%5BPipelineQueueManager%5D%28https%3A//github.com/langgenius/dify/blob/41a4a57d2e9e7d41fee5658f503bd350e4786c82/api/core/app/apps/pipeline/pipeline_queue_manager.py%23L22-L45%29%29%20already%20properly%20implement%20%60_publish%60%0A2.%20%60AppQueueManager%60%20shouldn%27t%20be%20instantiated%20directly%20anyway%E2%80%94the%20%60%40abstractmethod%60%20%2B%20%60raise%20NotImplementedError%60%20pattern%20indicates%20the%20intent%20was%20always%20for%20it%20to%20be%20abstract%0A3.%20Adding%20%60ABC%60%20inheritance%20would%20just%20add%20Python%27s%20built-in%20enforcement%20for%20what%27s%20already%20the%20expected%20behavior%0A%0AIf%20you%27d%20like%20to%20submit%20a%20PR%20for%20this%2C%20it%20should%20be%20a%20minimal%20change%E2%80%94just%20add%20%60ABC%60%20to%20the%20%5Bclass%20definition%5D%28https%3A//github.com/langgenius/dify/blob/41a4a57d2e9e7d41fee5658f503bd350e4786c82/api/core/app/apps/base_app_queue_manager.py%23L34%29%20and%20update%20the%20import.)&nbsp;[![Join Discord](https://img.shields.io/badge/join-5865F2?logo=discord&logoColor=white&label=)](https://go.dosu.dev/discord-bot)&nbsp;[![Share on X](https://img.shields.io/badge/X-share-black)](https://twitter.com/intent/tweet?text=%40dosu_ai%20helped%20me%20solve%20this%20issue!&url=https%3A//github.com/langgenius/dify/issues/32390)
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
Knowledge Pipeline
Proposal
WIP
cloud
dependencies
feat:webapp
good first issue
outdated
performance
pull-request
Mirrored from GitHub Pull Request
 python:uv
refactor
size:L
stale
status: accepting prs
status: blocked
status: in discussion
stt & tts
vulnerablity
web
wont-fix
🌊 feat:workflow
🌚 invalid
🐞 bug
👻 feat:rag
💪 enhancement
📚 documentation
🔨 feat:tools
🙋‍♂️ question
🙏 help wanted
🤔 cant-reproduce
🤖 feat:agent
No Label good first issue 🙏 help wanted
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
yindo
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: langgenius/dify#22247
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?