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

Fixed ClientSession initialization warning #1586

Closed
wants to merge 1 commit into from
Closed

Fixed ClientSession initialization warning #1586

wants to merge 1 commit into from

Conversation

AraHaan
Copy link
Contributor

@AraHaan AraHaan commented Feb 4, 2017

This fixes an issue with ClientSession throwing a warning when feeding
an event loop into it when calling it from a normal function. This
should help silence annoying warnings for libraries that "lazy"
initializes ClientSession and properly closes the session and handles
it. Now those libraries would have no need for useless bug reports.

What do these changes do?

Fixes the warning when calling a ClientSession instance with an event loop that is passed to the class.

Are there changes in behavior for the user?

None

Related issue number

#1468 (comment)

Checklist

  • I think the code is well written
  • Unit tests for the changes exist
  • Add yourself to CONTRIBUTORS.txt
    • The format is <Name> <Surname>.
    • Please keep alphabetical order, the file is sorted by names.
  • Add a new entry to CHANGES.rst
    • Choose any open position to avoid merge conflicts with other PRs.
    • Add a link to the issue you are fixing (if any) using #issue_number format at the end of changelog message. Use Pull Request number if there are no issues for PR or PR covers the issue only partially.

This fixes an issue with ClientSession throwing a warning when feeding
an event loop into it when calling it from a normal function. This
should help silence annoying warnings for libraries that "lazy"
initializes ClientSession and properly closes the session and handles
it. Now those libraries would have no need for useless bug reports.
@codecov-io
Copy link

codecov-io commented Feb 4, 2017

Codecov Report

Merging #1586 into master will increase coverage by <.01%.

@@            Coverage Diff             @@
##           master    #1586      +/-   ##
==========================================
+ Coverage   98.54%   98.54%   +<.01%     
==========================================
  Files          32       32              
  Lines        7265     7276      +11     
  Branches     1208     1210       +2     
==========================================
+ Hits         7159     7170      +11     
  Misses         61       61              
  Partials       45       45
Impacted Files Coverage Δ
aiohttp/client.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 b1c5f47...efc3737. Read the comment docs.

@fafhrd91
Copy link
Member

fafhrd91 commented Feb 6, 2017

add extra code just to satisfy tests? -1
proper solution is to create session inside task.

@Rapptz
Copy link

Rapptz commented Feb 6, 2017

Not that I'm condoning this PR, but @fafhrd91 the original PR was proposed to be modified: #1468 (comment)

@fafhrd91
Copy link
Member

fafhrd91 commented Feb 6, 2017

i think this PR is too much for andrew's comment.
in any case i addressed andrew's comment in aae6f8b

@fafhrd91 fafhrd91 closed this Feb 6, 2017
@AraHaan
Copy link
Contributor Author

AraHaan commented Feb 7, 2017

Ok, but you mind also adding my name in the CONTRIBUTORS.txt file to from this PR? @fafhrd91
And also this to CHANGES.rst:

- Fix ClientSession is not aware of TestClient #1499

Which was forgotten when the fix to #1499 got pushed.

@fafhrd91
Copy link
Member

fafhrd91 commented Feb 7, 2017

done

@AraHaan
Copy link
Contributor Author

AraHaan commented Feb 7, 2017

thanks.

@AraHaan AraHaan deleted the warnings_patch branch February 7, 2017 00:33
@lock
Copy link

lock bot commented Oct 29, 2019

This thread has been automatically locked since there has not been
any recent activity after it was closed. Please open a new issue for
related bugs.

If you feel like there's important points made in this discussion,
please include those exceprts into that new issue.

@lock lock bot added the outdated label Oct 29, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 29, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants