DAS-541: Pass in request session change

[gavindeiss]

Change to our fork of the canvasapi to allow us to pass in a request session that we can adjust pooling for

[Thetwam]

Hey @gavindeiss, thanks for your contribution!

In order to merge this change in, it will need to pass the GitHub Actions checks. That means it will need to pass the Flake8 linter, Black formatter, and have full test coverage.

It looks like there's a linting issue, but that might be fixed automatically if you run black. See our contributing guide for more details.

Additionally, the if statement on requester.py line 40 creates a branch that will probably need additional tests to have full test coverage.

What was the motivation behind this change? I think understanding that is key to writing a proper test for it.

Thanks again for your submission! I look forward to getting it merged in.

[gavindeiss]

Hi Matthew,

Thanks for the speedy reply. When running concurrent calls I was getting a lot of warnings about the connection pool being full similar to this https://stackoverflow.com/questions/53765366/urllib3-connectionpool-connection-pool-is-full-discarding-connection. Passing in a session with a custom HTTP adapter mounted that has more connection pools fixed the issue

adapter = requests.adapters.HTTPAdapter(pool_connections=100, pool_maxsize= 100) session.mount('https://', adapter) return Canvas(CANVAS_API_BASE_URL, access_token, session)

Thanks, Gavin

On Mon, Oct 3, 2022 at 12:03 PM Matthew Emond @.***> wrote:

Hey @gavindeiss https://github.com/gavindeiss, thanks for your contribution!

In order to merge this change in, it will need to pass the GitHub Actions checks. That means it will need to pass the Flake8 linter, Black formatter, and have full test coverage.

It looks like there's a linting issue, but that might be fixed automatically if you run black. See our contributing guide https://github.com/ucfopen/canvasapi/blob/develop/.github/CONTRIBUTING.md#running-code-style-checks for more details.

Additionally, the if statement on requester.py line 40 creates a branch that will probably need additional tests to have full test coverage.

What was the motivation behind this change? I think understanding that is key to writing a proper test for it.

Thanks again for your submission! I look forward to getting it merged in.

— Reply to this email directly, view it on GitHub https://github.com/ucfopen/canvasapi/pull/556#issuecomment-1265686028, or unsubscribe https://github.com/notifications/unsubscribe-auth/AXZTRRD64TUBZI4W7QGOX33WBL7WRANCNFSM6AAAAAAQ3W4TWQ . You are receiving this because you were mentioned.Message ID: @.***>

[Thetwam]

Hey @gavindeiss, I've done a little work on my end that resolves the formatting issues I listed above, and adds tests for custom sessions. I am unable to push my changes to your branch.

Could you please enable the "Allow edits and access to secrets by maintainers" checkbox on this pull request? Thanks!

[Thetwam]

Hey @gavindeiss, I'd like to incorporate your work into the library. Could you please allow edits to your branch, so I can make some final tweaks before merging? Thanks!

GitHub's docs explain how to do this