Skip to content

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.

4 participants