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

Fix progress bar still capturing stderr after exception #16

Merged
merged 12 commits into from
Nov 10, 2019

Conversation

Neraste
Copy link
Member

@Neraste Neraste commented Oct 13, 2019

Fixes #15, fixes #22.

@Neraste Neraste added the bug Something isn't working label Oct 13, 2019
@Neraste Neraste self-assigned this Oct 13, 2019
@codecov
Copy link

codecov bot commented Oct 13, 2019

Codecov Report

Merging #16 into develop will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop      #16      +/-   ##
===========================================
+ Coverage    99.54%   99.54%   +<.01%     
===========================================
  Files            9        9              
  Lines          439      442       +3     
===========================================
+ Hits           437      440       +3     
  Misses           2        2
Impacted Files Coverage Δ
src/dakara_base/progress_bar.py 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6de0760...93c7678. Read the comment docs.

@Neraste Neraste requested a review from Nadeflore November 3, 2019 14:01
Copy link
Member

@Nadeflore Nadeflore left a comment

Choose a reason for hiding this comment

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

I remember this issue, so using a context manager should solve the problem.
Didn't test it, but it makes sense.

@Neraste
Copy link
Member Author

Neraste commented Nov 3, 2019

I would like to open an issue on the progress bar repository.

@Neraste
Copy link
Member Author

Neraste commented Nov 3, 2019

Looks like a similar issue was submitted, and that the progress bar can be already used as a context manager to prevent the problem. So this PR is redundant, but not the final use of the different progress bars. I will check that and adapt the code.

@Neraste Neraste added the enhancement New feature or request label Nov 4, 2019
@Neraste
Copy link
Member Author

Neraste commented Nov 4, 2019

I adapted the code, it is better to used the already provided secure mechanism.

@Neraste Neraste requested a review from Nadeflore November 4, 2019 06:09
@Neraste Neraste merged commit 6eef726 into develop Nov 10, 2019
@Neraste Neraste deleted the fix/15_progress_bar branch November 10, 2019 07:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add percentage in progress bar Unknown exceptions are not displayed when progress bar is used
2 participants