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

Return Correct Error on Context Cancellation #30

Merged
merged 7 commits into from
May 19, 2020

Conversation

patrick-ogrady
Copy link
Contributor

@patrick-ogrady patrick-ogrady commented May 18, 2020

Motivation

  • If context is cancelled, retry methods in the fetcher package return an incorrect error (exhausted retries instead of context error).
  • If context is cancelled, the reconciler can silence the ctx.Err() and return nil instead.

Solution

Return the correct error.

@coveralls
Copy link

coveralls commented May 18, 2020

Pull Request Test Coverage Report for Build 1357

  • 60 of 68 (88.24%) changed or added relevant lines in 5 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-4.08%) to 64.462%

Changes Missing Coverage Covered Lines Changed/Added Lines %
reconciler/reconciler.go 0 8 0.0%
Totals Coverage Status
Change from base Build 1285: -4.08%
Covered Lines: 1277
Relevant Lines: 1981

💛 - Coveralls

@patrick-ogrady patrick-ogrady force-pushed the patrick/improve-logging branch 2 times, most recently from f117a0d to b60a722 Compare May 19, 2020 01:15
@patrick-ogrady
Copy link
Contributor Author

Decrease in coverage is deceptive because tests were added to areas of the code that were previously untested.

@patrick-ogrady patrick-ogrady merged commit 620dbd4 into master May 19, 2020
@patrick-ogrady patrick-ogrady deleted the patrick/improve-logging branch May 19, 2020 14:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants