Skip to content

chore: remove broad Exception catching from config.py #2212

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

Merged
merged 1 commit into from
Aug 7, 2022
Merged

Conversation

JohnVillalovos
Copy link
Member

Change "except Exception:" catching to more granular exceptions.

A step in enabling the "broad-except" check in pylint.

@JohnVillalovos JohnVillalovos marked this pull request as draft August 2, 2022 05:45
@JohnVillalovos JohnVillalovos marked this pull request as ready for review August 2, 2022 06:27
@JohnVillalovos JohnVillalovos requested a review from nejch August 2, 2022 16:00
@nejch
Copy link
Member

nejch commented Aug 3, 2022

Thanks @JohnVillalovos! Just 2 questions.

FWIW I recently realized we are rolling our own homebrew version of configparser (with all this try/except spaghetti) when python3's (and previously SafeConfigParser) parser actually handles section- vs. default defined options natively.

But probably not worth migrating that breaking change now that tomllib is coming to the stdlib and might be more attractivein the future to bring the config format in line with trends in the python world.

@codecov-commenter
Copy link

codecov-commenter commented Aug 4, 2022

Codecov Report

Merging #2212 (0abc90b) into main (9aecc9e) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #2212      +/-   ##
==========================================
- Coverage   95.43%   95.43%   -0.01%     
==========================================
  Files          81       81              
  Lines        5369     5368       -1     
==========================================
- Hits         5124     5123       -1     
  Misses        245      245              
Flag Coverage Δ
api_func_v4 81.35% <75.00%> (+0.03%) ⬆️
cli_func_v4 82.97% <90.00%> (-0.08%) ⬇️
unit 87.27% <100.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
gitlab/config.py 100.00% <100.00%> (ø)

Change "except Exception:" catching to more granular exceptions.

A step in enabling the "broad-except" check in pylint.
@nejch
Copy link
Member

nejch commented Aug 7, 2022

Cool, I think we can get this in and double check if any new errors are thrown before the 28th :)

@nejch nejch merged commit 70e67bf into main Aug 7, 2022
@nejch nejch deleted the jlvillal/config branch August 7, 2022 09:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants