Codecov Report

Merging #1296 (cab343c) into master (9d6c188) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1296   +/-   ##
=======================================
  Coverage   78.26%   78.26%           
=======================================
  Files          12       12           
  Lines        2894     2894           
=======================================
  Hits         2265     2265           
  Misses        629      629           

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

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 9d6c188...cab343c. Read the comment docs.

We found this PR after we tried including gitlab-cli in a GitLab CI/CD pipeline job. This PR looks like it will achieve what we wanted to do (drive the GitLab API without creating a config file in each job) 🎉

One outstanding wishlist item we had was if the tool could look for the same environment variables that are set inside a GitLab CI/CD job, i.e. CI_JOB_TOKEN, CI_SERVER_URL and so on 1. If the tool shared these varaibles, it could authenticate with the GitLab instance without any extra code written in CI/CD jobs - is that something that has already been considered?

We found this PR after we tried including gitlab-cli in a GitLab CI/CD pipeline job. This PR looks like it will achieve what we wanted to do (drive the GitLab API without creating a config file in each job) 🎉

One outstanding wishlist item we had was if the tool could look for the same environment variables that are set inside a GitLab CI/CD job, i.e. CI_JOB_TOKEN, CI_SERVER_URL and so on 1. If the tool shared these varaibles, it could authenticate with the GitLab instance without any extra code written in CI/CD jobs - is that something that has already been considered?

Yes, part of this was meant to implement zero-config for CI jobs. But I need to maybe tweak and add tests to really check all scenarios (for some endpoints, a PAT is needed, so we should allow overriding in a sensible precedence between cli args/env/config files/CI env). Currently the precedence planned is:

I'll try to get back to this PR in the coming days :)

Yes, part of this was meant to implement zero-config for CI jobs. But I need to maybe tweak and add tests to really check all scenarios (for some endpoints, a PAT is needed, so we should allow overriding in a sensible precedence between cli args/env/config files/CI env). Currently the precedence planned is:

python-gitlab/gitlab/__init__.py

Lines 224 to 232 in cab343c

	1. Explicitly provided CLI arguments,
	2. Environment variables,
	3. Configuration files:
	a. explicitly defined config files:
	i. via the `--config-file` CLI argument,
	ii. via the `PYTHON_GITLAB_CFG` environment variable,
	b. user-specific config file,
	c. system-level config file,
	4. Environment variables always present in CI (CI_SERVER_URL, CI_JOB_TOKEN).

Aha. I'm blind 👀

That does indeed look like exactly the behaviour we wanted.

Codecov Report

Merging #1296 (13467c5) into master (ce4bc0d) will increase coverage by 0.03%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #1296      +/-   ##
==========================================
+ Coverage   91.57%   91.61%   +0.03%     
==========================================
  Files          74       74              
  Lines        4263     4281      +18     
==========================================
+ Hits         3904     3922      +18     
  Misses        359      359              

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

What do you think about using argparse.ArgumentParser..add_mutually_exclusive_group() ?

For example:

private_token_group = parser.add_mutually_exclusive_group()
private_token_group.add_argument(
        "--private-token",
        help=("GitLab private token"),
        required=False,
    )
private_token_group.add_argument(
        "--private-token-env-var",
        help=("Name of environment variable containing GitLab private token"),
        required=False,
    )

But now that I look at the code I'm not sure where to do the processing to load the environment variable 😟

I'll just leave this comment anyway and maybe it gives you an idea 🤷‍♂️

.add_mutually_exclusive_group()

Thanks! Yes, I think it might be useful (not for arg/var exclusive I think but maybe for different types of tokens). But yes sometimes we want to validate those a bit later, because things like CI_JOB_TOKEN might always be present 🤦

I think we just need to write a bunch of tests for expected behaviors in all the possible combinations, and write the code based on that.

@JohnVillalovos took almost a year to get back to this, but ready for another look 😅