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

Make pylint overgeneral-exceptions check actually work. #447

Merged
merged 4 commits into from
Aug 13, 2024

Conversation

adk-swisstopo
Copy link
Member

@adk-swisstopo adk-swisstopo commented Aug 13, 2024

This fixes a bug in the pylintrc configuration. This also "fix" some broad exceptions either by using more specific ones or by disabling the check when I couldn't quickly find a more appropriate exception.

This disable "broad-exception-raised" for some specific instances
of "raise Exception" which don't have an obvious better more specific
exception. This allows the broad-exception-raised pylintrc misconfiguration
to be fixed without breaking the build.
This allows re-enabling the pylint check for broad exceptions.
We pick LookupError because that is more specific than Exception and it
seems to roughly match the semantic. It is possible serializers.ValidationError
might be more appropriate.
This allows enabling pylint check for broad exceptions. We pick EnvironmentError
because it is more specific than Exception and seems to match the semantic.
Something even more specific might be more appropriate.
Before this change, we see the following messages when running `make lint`:

    pylint: Command line or configuration file:1: UserWarning: 'BaseException' is not a proper value for the 'overgeneral-exceptions' option. Use fully qualified name (maybe 'builtins.BaseException' ?) instead. This will cease to be checked at runtime when the configuration upgrader is released.
    pylint: Command line or configuration file:1: UserWarning: 'Exception' is not a proper value for the 'overgeneral-exceptions' option. Use fully qualified name (maybe 'builtins.Exception' ?) instead. This will cease to be checked at runtime when the configuration upgrader is released.
    pylint: Command line or configuration file:1: UserWarning: 'StandardError' is not a proper value for the 'overgeneral-exceptions' option. Use fully qualified name (maybe 'builtins.StandardError' ?) instead. This will cease to be checked at runtime when the configuration upgrader is released.

This change resolves this and broad exceptions are effectively caught.
@adk-swisstopo adk-swisstopo requested a review from schtibe August 13, 2024 13:11
@github-actions github-actions bot added the bug label Aug 13, 2024
@@ -171,7 +171,7 @@ def _get_boto_access_kwargs(s3_bucket: AVAILABLE_S3_BUCKETS = AVAILABLE_S3_BUCKE
needed_env_vars = ['AWS_ROLE_ARN', 'AWS_WEB_IDENTITY_TOKEN_FILE']
for env_var in needed_env_vars:
if env_var not in os.environ:
raise Exception(
raise EnvironmentError(
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice!

@adk-swisstopo adk-swisstopo merged commit 13bf391 into develop Aug 13, 2024
5 checks passed
@adk-swisstopo adk-swisstopo deleted the fix-PB-883-lint branch August 13, 2024 13:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants