-
Notifications
You must be signed in to change notification settings - Fork 281
chore: improve type checking behavior #1470
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,25 +1,28 @@ | ||
| import logging | ||
| from typing import Optional | ||
| from typing import Optional, TYPE_CHECKING | ||
|
|
||
| from aiohttp import web | ||
|
|
||
| from slack_bolt.adapter.aiohttp import to_bolt_request, to_aiohttp_response | ||
| from slack_bolt.response import BoltResponse | ||
| from slack_bolt.util.utils import get_boot_message | ||
|
|
||
| if TYPE_CHECKING: | ||
| from slack_bolt.app.async_app import AsyncApp | ||
|
Comment on lines
+10
to
+11
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🌟 praise: Love to find circular imports avoided without a sacrifice to sureness! |
||
|
|
||
|
|
||
| class AsyncSlackAppServer: | ||
| port: int | ||
| path: str | ||
| host: str | ||
| bolt_app: "AsyncApp" # type: ignore[name-defined] | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🪓 praise: These comments have been confusing to me. Thanks for finding pattern of fixes! |
||
| bolt_app: "AsyncApp" | ||
| web_app: web.Application | ||
|
|
||
| def __init__( | ||
| self, | ||
| port: int, | ||
| path: str, | ||
| app: "AsyncApp", # type: ignore[name-defined] | ||
| app: "AsyncApp", | ||
| host: Optional[str] = None, | ||
| ): | ||
| """Standalone AIOHTTP Web Server. | ||
|
|
@@ -34,7 +37,7 @@ def __init__( | |
| self.port = port | ||
| self.path = path | ||
| self.host = host if host is not None else "0.0.0.0" | ||
| self.bolt_app: "AsyncApp" = app # type: ignore[name-defined] | ||
| self.bolt_app: "AsyncApp" = app | ||
| self.web_app = web.Application() | ||
| self._bolt_oauth_flow = self.bolt_app.oauth_flow | ||
| if self._bolt_oauth_flow: | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📺 question: Are we wanting to split this from
scriptsmoving forward?👾 ramble: I find bash scripts can be most portable but these workflows seem to make use of ordered dependencies?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yess this aims to take advantage of ordered dependencies, ensuring that "synchronous" Bolt will never depend on "asynchronous" dependencies
I would like to keep the bash scripts in sync but I think this has some benefit to ensure we never accidentally depend on asynchronous dependencies