korbit-ai[bot] commented on code in PR #33095: URL: https://github.com/apache/superset/pull/33095#discussion_r2039089451
########## superset/app.py: ########## @@ -14,11 +14,21 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. +from __future__ import annotations import logging import os +import sys from typing import cast, Iterable, Optional -from wsgiref.types import StartResponse, WSGIApplication, WSGIEnvironment + +if sys.version_info >= (3, 11): + from wsgiref.types import StartResponse, WSGIApplication, WSGIEnvironment +else: + from typing import TYPE_CHECKING + + if TYPE_CHECKING: + from _typeshed.wsgi import StartResponse, WSGIApplication, WSGIEnvironment Review Comment: ### Runtime Missing WSGI Types <sub></sub> <details> <summary>Tell me more</summary> ###### What is the issue? The WSGI types are only imported during type checking for Python versions <= 3.10, but these types are actually used at runtime in the AppRootMiddleware class. ###### Why this matters This will cause a runtime NameError when running the code with Python <= 3.10 because the required WSGI types won't be available during execution. ###### Suggested change ∙ *Feature Preview* Move the WSGI type imports outside the TYPE_CHECKING block for Python <= 3.10: ```python if sys.version_info >= (3, 11): from wsgiref.types import StartResponse, WSGIApplication, WSGIEnvironment else: from _typeshed.wsgi import StartResponse, WSGIApplication, WSGIEnvironment ``` ###### Provide feedback to improve future suggestions [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/e08cd26c-5fa5-4084-878f-46cfca740e06/upvote) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/e08cd26c-5fa5-4084-878f-46cfca740e06?what_not_true=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/e08cd26c-5fa5-4084-878f-46cfca740e06?what_out_of_scope=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/e08cd26c-5fa5-4084-878f-46cfca740e06?what_not_in_standard=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/e08cd26c-5fa5-4084-878f-46cfca740e06) </details> <sub> 💬 Looking for more details? Reply to this comment to chat with Korbit. </sub> <!--- korbi internal id:16ea3d29-0ade-4644-bd0e-2623d8eb3d25 --> [](16ea3d29-0ade-4644-bd0e-2623d8eb3d25) ########## superset/app.py: ########## @@ -14,11 +14,21 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. +from __future__ import annotations import logging import os +import sys from typing import cast, Iterable, Optional -from wsgiref.types import StartResponse, WSGIApplication, WSGIEnvironment + +if sys.version_info >= (3, 11): + from wsgiref.types import StartResponse, WSGIApplication, WSGIEnvironment +else: + from typing import TYPE_CHECKING + + if TYPE_CHECKING: + from _typeshed.wsgi import StartResponse, WSGIApplication, WSGIEnvironment Review Comment: ### Unnecessarily nested version-dependent imports <sub></sub> <details> <summary>Tell me more</summary> ###### What is the issue? The version-dependent import logic is nested and split across multiple conditions, making it harder to follow the code flow. ###### Why this matters The nested conditions for imports increase cognitive load and make the import section less scannable. Type checking condition could be combined with version check for better readability. ###### Suggested change ∙ *Feature Preview* ```python if TYPE_CHECKING: if sys.version_info >= (3, 11): from wsgiref.types import StartResponse, WSGIApplication, WSGIEnvironment else: from _typeshed.wsgi import StartResponse, WSGIApplication, WSGIEnvironment else: if sys.version_info >= (3, 11): from wsgiref.types import StartResponse, WSGIApplication, WSGIEnvironment else: from _typeshed.wsgi import StartResponse, WSGIApplication, WSGIEnvironment ``` ###### Provide feedback to improve future suggestions [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/3a3fe6f9-b37a-4e8d-881e-65a18b927c24/upvote) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/3a3fe6f9-b37a-4e8d-881e-65a18b927c24?what_not_true=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/3a3fe6f9-b37a-4e8d-881e-65a18b927c24?what_out_of_scope=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/3a3fe6f9-b37a-4e8d-881e-65a18b927c24?what_not_in_standard=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/3a3fe6f9-b37a-4e8d-881e-65a18b927c24) </details> <sub> 💬 Looking for more details? Reply to this comment to chat with Korbit. </sub> <!--- korbi internal id:8918ff32-b830-4e2c-b9cc-5ae84f4fa324 --> [](8918ff32-b830-4e2c-b9cc-5ae84f4fa324) -- 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]
