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

aiohttp-session is not required #107

Merged
merged 3 commits into from
Oct 17, 2017

Conversation

oleksandr-kuzmenko
Copy link
Contributor

Fixed this case:

$ pip install aiohttp-security
...
$ python3
>>> import aiohttp_security
...
ImportError: No module named 'aiohttp_session'

Another solution — throw error during SessionIdentityPolicy init

@asvetlov
Copy link
Member

Sorry for inactivity but I prefer the second alternative.

@oleksandr-kuzmenko oleksandr-kuzmenko changed the title aiohttp-session is required aiohttp-session is not required Oct 10, 2017
@oleksandr-kuzmenko
Copy link
Contributor Author

@asvetlov what do you think about this PR?

@asvetlov
Copy link
Member

Please add # pragma: no cover for cases where aiohttp-session is not installed

@codecov
Copy link

codecov bot commented Oct 12, 2017

Codecov Report

Merging #107 into master will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #107      +/-   ##
==========================================
+ Coverage   97.02%   97.03%   +0.01%     
==========================================
  Files          10       10              
  Lines         403      405       +2     
  Branches        8        8              
==========================================
+ Hits          391      393       +2     
  Misses         10       10              
  Partials        2        2
Impacted Files Coverage Δ
aiohttp_security/session_identity.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 a46ca6e...2dc126b. Read the comment docs.

@oleksandr-kuzmenko
Copy link
Contributor Author

@asvetlov done

@jettify
Copy link
Member

jettify commented Oct 17, 2017

@asvetlov looks good to me, if you are ok with changes, I can merge

@asvetlov asvetlov merged commit aef24fa into aio-libs:master Oct 17, 2017
@asvetlov
Copy link
Member

Thanks

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.

3 participants