add **connection_pool_kw to Request() constructor

[NiklasRosenstein]

Adds **connection_pool_kw to the Request() class that can be used to override the default values picked in the Request class or pass additional parameters.

Original use case #2557

Checklist for PRs

• Added .. versionadded:: version, .. versionchanged:: version or .. deprecated:: version to the docstrings for user facing changes (for methods/class descriptions, arguments and attributes)
• Created new or adapted existing unit tests
• Added myself alphabetically to AUTHORS.rst (optional)

If the PR contains API changes (otherwise, you can delete this passage)

• Updated classes:
  • Request.__init__ now accepts **connection_pool_kw

[NiklasRosenstein]

Hey @ed738 , thanks for the approval.

Seems like a test is failing but I don't quite see how it is connected to the change in this PR. Do you have an idea?

    def test_clean_deprecation_warning_polling(self, recwarn, updater, monkeypatch):
        monkeypatch.setattr(updater.bot, 'set_webhook', lambda *args, **kwargs: True)
        monkeypatch.setattr(updater.bot, 'delete_webhook', lambda *args, **kwargs: True)
        # prevent api calls from @info decorator when updater.bot.id is used in thread names
        monkeypatch.setattr(updater.bot, '_bot', User(id=123, first_name='bot', is_bot=True))
        monkeypatch.setattr(updater.bot, '_commands', [])
    
        updater.start_polling(clean=True)
        updater.stop()
        for msg in recwarn:
            print(msg)
>       assert len(recwarn) == 2
E       assert 3 == 2
E         +3
E         -2

[NiklasRosenstein]

Maybe the other one was a flake, latest run resulted in

telegram/utils/request.py:121: error: Function is missing a type annotation for one or more arguments  [no-untyped-def]
Found 1 error in 1 file (checked 146 source files)

Fixed in latest commit

[Bibo-Joshi]

Hi. Thanks for the PR and sorry for getting back to it late.

• test_clean_deprecation_warning_polling is indeed sometimes flaky.
• pre-commit seems to be happy now
• It would be good to have a test for this. I know that we don't test Request too much (test_request.py is not long …), but it's probably possible to at least make sure that the new kwargs are properly passed to the pool manager. If you haven't worked with pytest before, let me know.

[NiklasRosenstein]

@Bibo-Joshi , I addressed your comments. Please take another look

[Bibo-Joshi]

Hi. The code itself looks okay now, but I'll still need some unit test for this before merging ;)

[Bibo-Joshi]

@ed738 please stop producing noise on this PR (or other threads). If you don't, you will be reported to GitHub and blocked from our organization.

[NiklasRosenstein]

Hey @Bibo-Joshi , only picking this up again now. How would you like to test this? We could try passing in a parameter that is expected to be forwarded to the connection pool, then assert that the parameter is set as expected.

[Bibo-Joshi]

We could try passing in a parameter that is expected to be forwarded to the connection pool, then assert that the parameter is set as expected.

Yes, that sounds reasonable. Please make sure to cover all cases, where the kwargs would be used (IISC it's 3 cases).

BTW I rebased to the v14 branch, as we've started developing for v14 and all PRs will go into that branch now.

[Bibo-Joshi]

@8136800738 please stop producing noise on this PR (or other threads). If you don't, you will be reported to GitHub and blocked from our organization.

[Bibo-Joshi]

Closing due to inactivity and also because this is likely to be irrelevant after #2731