Skip to content
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

chore: enable raise-without-from-inside-except ruff rule #1753

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

kyujin-cho
Copy link
Member

This will help investigating root cause of the error more easier when inspecting the error log.

Checklist: (if applicable)

  • Milestone metadata specifying the target backport version

@kyujin-cho kyujin-cho added skip:changelog Make the action workflow to skip towncrier check impact:invisible This change is invisible to users (internal changes). labels Dec 2, 2023
@kyujin-cho kyujin-cho added this to the 24.03 milestone Dec 2, 2023
@kyujin-cho kyujin-cho self-assigned this Dec 2, 2023
@github-actions github-actions bot added comp:client Related to Client component comp:manager Related to Manager component comp:storage-proxy Related to Storage proxy component size:XL 500~ LoC labels Dec 2, 2023
@@ -143,7 +143,7 @@ async def route_structure_fallback_middleware(
structure_pointer = self.route_structure
for component in components:
if structure_pointer.get(component) is None:
raise web.HTTPNotFound
raise web.HTTPNotFound from None
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case, I think we should simply reraise the original exception.

Suggested change
raise web.HTTPNotFound from None
raise

Copy link
Member

@achimnol achimnol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the auto-generated as ex expressions, I think it would be better to unify the style: as e.

According to the official doc, Python automatically attach the chaining information when the raise statement is inside an except block.

We should force this rule only when the raise statement is not inside the except block.

@@ -350,9 +350,9 @@ async def _download_file(
chunk_size
)
except asyncio.TimeoutError:
raise TryAgain
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couldn't we just simply raise a "root" exception instead of specifying from None always for brevity?

@kyujin-cho kyujin-cho added the action:on hold Hold it. Wait for the restart. label Mar 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
action:on hold Hold it. Wait for the restart. comp:client Related to Client component comp:manager Related to Manager component comp:storage-proxy Related to Storage proxy component impact:invisible This change is invisible to users (internal changes). size:XL 500~ LoC skip:changelog Make the action workflow to skip towncrier check
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants