Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix authorization status checker to check the last request before StopIteration #148

Merged
merged 3 commits into from
Mar 15, 2020
Merged

Conversation

rozgonik
Copy link
Contributor

Thank you for contributing to sewer.
Every contribution to sewer is important to us.
You may not know it, but you have just contributed to making the world a more safer and secure place.

What(What have you changed?)

Changed the order of checks.

Why(Why did you change it?)

Found a bug where the last GET response was not evaluated (but logged) before raising a StopIteration exception. Although the last response had a "valid" status, sewer still raised the exception.

This way the while loop does not terminate prematurely before checking the last request's status.

@codecov
Copy link

codecov bot commented Mar 11, 2020

Codecov Report

Merging #148 into master will decrease coverage by 0.1%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #148      +/-   ##
==========================================
- Coverage   86.53%   86.42%   -0.11%     
==========================================
  Files          14       14              
  Lines         943      943              
==========================================
- Hits          816      815       -1     
- Misses        127      128       +1
Impacted Files Coverage Δ
sewer/client.py 92.07% <100%> (-0.34%) ⬇️

@mmaney
Copy link
Collaborator

mmaney commented Mar 13, 2020

+1 on fixing the problem of throwing away the last result, which this should do.

-1 on the disguised counting loop that doubtless was the cause of this error in the first place. But that could be a refactoring for another time.

@komuw
Copy link
Owner

komuw commented Mar 14, 2020

@mmaney are you happy with this PR?
If you are, you please add an approval. You can also go ahead and squash merge

@mmaney mmaney merged commit 43c3c8e into komuw:master Mar 15, 2020
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants