Noticedanother MR that has had a failed pipeline for a while. It also did not have documentation. It made sense to make the small change from scratch and update documentation.

I have run the test locally (for my python version). I have not generated the documentation and run a visual check on the page itself yet.

Does anyone know what it would take for the pipelines to run and provide feedback? Do the tests and linters not run on the PR?

Codecov Report

Merging#1565 (3b1d3a4) intomaster (a00ec87) willincrease coverage by0.02%.
The diff coverage is100.00%.

@@            Coverage Diff             @@##           master    #1565      +/-   ##==========================================+ Coverage   91.16%   91.18%   +0.02%==========================================  Files          74       74                Lines        4177     4188      +11     ==========================================+ Hits         3808     3819      +11  Misses        369      369

Flags with carried forward coverage won't be shown.Click here to find out more.

Noticedanother MR that has had a failed pipeline for a while. It also did not have documentation. It made sense to make the small change from scratch and update documentation.

I have run the test locally (for my python version). I have not generated the documentation and run a visual check on the page itself yet.

Does anyone know what it would take for the pipelines to run and provide feedback? Do the tests and linters not run on the PR?

Thanks for the contribution@javatarz! I've just triggered the workflow, it needs to be approved for first-time contributors.

I'll take a look at this later tonight but yes adding a basic test would generally be good (if it makes sense here - maybe just to confirm the correct option is used depending on global/overrides) :) Thanks!

I had looked for tests around this and there were none. I can see a test forhttp_request. I'll add a couple of tests for

1. verifying the default behaviour (whenretry_transient_errors is not provided)
2. verifying howretry_transient_errors works (missing test case)
3. verifying the default value gets applied
4. verifying that the default value can be overridden

Please don't merge this MR, I'll be back in a few hours with tests!

I have added the missing tests and update the documentation a bit. Here's what the documentation looks like now (after rendering)

@nejch: This one's ready for review again. It needs a nudge for running the checks on the pipeline. Tests passed locally on py39.

Hey@javatarz finally had a chance to look at this properly, thanks again, looks great!

This looks like a good candidate to be added to global (or per-server) config in the configuration file to keep it consistent with existing global options. You can take a look at the user agent (the last option added before yours) as inspiration if it helps:https://github.com/python-gitlab/python-gitlab/pull/1277/files

(I realizeconfig.py could de-duplicate a bit..). We can also do it in a follow-up, just let me know. thanks!

Sorry for the late response. Just had a look atconfig.py and it makes a lot of sense to add theretry_transient_errors to be supported through the configuration mechanism. Will push updates soon. (if you mark this as draft, you will have to trigger the pipeline again 😂)

@nejch: I have added changes to allow config values to be set (both globally and per gitlab connection).

Used mypy typing in test utlity functions which isn't a pattern on the file. I cannot run mypy locally (becauseyour fix for mypy has not been merged yet 😞). Not sure if I have broken something.

Do you mind approving the pipeline run for me once again so I can confirm if everything is ok?
Let me know if there's any other change necessary before we can get this merged. 😃

@nejch: I have added changes to allow config values to be set (both globally and per gitlab connection).

Used mypy typing in test utlity functions which isn't a pattern on the file. I cannot run mypy locally (becauseyour fix for mypy has not been merged yet disappointed). Not sure if I have broken something.

Do you mind approving the pipeline run for me once again so I can confirm if everything is ok?
Let me know if there's any other change necessary before we can get this merged. smiley

Thank you@javatarz! I see mypy is not happy :) the hook fix should be ready soon. maybe you can check how it's done for the default scenario inssl_verify withgetboolean() if it helps?

Can confirm,mypy runs fine well locally! Thanks@nejch!

The build has passed locally for Python 3.9. Should be fine on other versions too.Do you mind triggering the pipeline for me please? Just noticed that someone triggered it 😄

Seems like the test failure is unrelated. I'm trying to run functional tests locally to confirm.

Seems like the test failure is unrelated. I'm trying to run functional tests locally to confirm.

The functional tests can be a little flaky sometimes, although I thought this was mostly fixed. I'm re-running now.

Dang, they passed now. Should we merge this one now or should we disable that flaky test first?

Are there any other steps before this change can be accepted@nejch,@JohnVillalovos?

Picked names off of the recent people to merge/commit things

@javatarz Can the git commits be cleaned up? In particular the "merge" commits. It would be nicer to have a cleaner git history, IMHO.

@JohnVillalovos: can you have another look? I have squashed the commits into a single commit now.

Thought that's a bug with my local setup. Wish the pipeline automatically ran after commits so I could get this feedback quicker.

@JohnVillalovos,@max-wittig,@nejch : Do you mind re-triggering the pipeline for me? Please let me know if there's any other review comments.

@nejch: is it possible to publish a new version of this artifact to pypi? I'd like to use this feature in multiple projects where I use this library. 😄

Thanks for the ping@javatarz! I would like to and usually we have a release every 28th but I've disabled it because we have#1520 next, and we're dropping support for 3.6 so we thought we'd release it with 3.6 EOL, along with other breaking changes in the library.

I could maybe try cherry-picking non-breaking changes into a 2.11.0 or a pre-release branch though.

@nejch We're in a similar boat, would love to see a release cut with this particular change.