Skip to content
This repository was archived by the owner on Jan 18, 2025. It is now read-only.

Check whether a credential loaded from a store is expired before using it.#158

Closed
mattmoor wants to merge 1 commit intogoogleapis:masterfrom
mattmoor:master
Closed

Check whether a credential loaded from a store is expired before using it.#158
mattmoor wants to merge 1 commit intogoogleapis:masterfrom
mattmoor:master

Conversation

@mattmoor
Copy link
Copy Markdown

@mattmoor mattmoor commented Apr 3, 2015

No description provided.

@nathanielmanistaatgoogle
Copy link
Copy Markdown
Contributor

nathanielmanistaatgoogle commented Apr 3, 2015

What hope is there for test coverage for this change?

@mattmoor mattmoor force-pushed the master branch 2 times, most recently from af59aa3 to b930dde Compare April 4, 2015 16:01
@coveralls
Copy link
Copy Markdown

coveralls commented Apr 4, 2015

Coverage Status

Coverage remained the same at 64.8% when pulling b930dde on mattmoor:master into 0a6241c on google:master.

@coveralls
Copy link
Copy Markdown

coveralls commented Apr 4, 2015

Coverage Status

Coverage remained the same at 64.8% when pulling 17ad129 on mattmoor:master into 0a6241c on google:master.

@coveralls
Copy link
Copy Markdown

coveralls commented Apr 4, 2015

Coverage Status

Coverage remained the same at 64.8% when pulling 88fa745 on mattmoor:master into 0a6241c on google:master.

@mattmoor
Copy link
Copy Markdown
Author

mattmoor commented Apr 4, 2015

Technically you were already covering it with bad tests. ;-)

I fixed the break and added testing of an actual refresh.

@mattmoor
Copy link
Copy Markdown
Author

mattmoor commented Apr 10, 2015

ping?

@nathanielmanistaatgoogle
Copy link
Copy Markdown
Contributor

nathanielmanistaatgoogle commented Apr 20, 2015

Please tweak your commit message to be in accordance with http://chris.beams.io/posts/git-commit/#seven-rules.

Code looks reasonable to me, but @anthmgoogle or @soltanmm, would you mind being a second pair of eyes and also reviewing?

@mattmoor
Check expiration of credentials loaded from store
f3c20ce 
Today the _refresh logic lazily loads a credential from its store during
 _refresh, but blindly trusts its validity (no expiry check).

This adds a check that the stored credential has not expired before
updating from the cache, falling back on the existing uncached path.
@mattmoor
Copy link
Copy Markdown
Author

mattmoor commented Apr 21, 2015

Should be done.

@nathanielmanistaatgoogle
Copy link
Copy Markdown
Contributor

nathanielmanistaatgoogle commented Apr 21, 2015

Commit message is much better; thanks.

@coveralls
Copy link
Copy Markdown

coveralls commented Apr 21, 2015

Coverage Status

Coverage remained the same at 64.94% when pulling f3c20ce on mattmoor:master into 22a532d on google:master.

@thobrla
Copy link
Copy Markdown
Contributor

thobrla commented May 11, 2015

Created a updated pull request based on this logic at #173 that also covers the case where the token in the store is about to expire.

@craigcitro craigcitro mentioned this pull request May 11, 2015
Merged
@craigcitro
Copy link
Copy Markdown
Contributor

craigcitro commented May 16, 2015

this was actually included in #173 (i think we needed to mention that in the commit to magically close this?).

@craigcitro craigcitro closed this May 16, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

cla: yes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@mattmoor @nathanielmanistaatgoogle @coveralls @thobrla @craigcitro @googlebot