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

refactor(config): enable default environment token per hvcs #774

Conversation

codejedi365
Copy link
Contributor

@codejedi365 codejedi365 commented Dec 17, 2023

Purpose

Update the configuration parser to optionally default to the VCS specific token

Rationale

In preparation to facilitate the request of #734, I need to find a way to solve the missing token when the remote.type is none. This led me to determining that it would be helpful if we looked for the default token names for each of the VCS. This refactor enables the configuration to be aware of each VCS's default.

Current downside is that Gitea does not provide a pre-authed CI-like token based on this discussion. I just guessed to use GITEA_TOKEN. I could potentially have it be empty instead, which would force the user to provide it.

Consideration also should be made for GitLab as GitLab's default token CI_JOB_TOKEN has permissions problems with python-semantic-release. The fact that python-semantic-release uses git push both commits and tags are both unauthorized actions by a CI_JOB_TOKEN. This means users are forced to provide a PAT as that is how we mark versions. What the users define as that token name is up to them. My guess would be to use GITLAB_TOKEN but maybe we need to force them to be specific here also. Curious on your thoughts.

Separately, there is still some investigation that I'm doing here to hopefully find a better implementation for GitLab.

One of the other issues (besides for none) that I have with the current implementation is that if the user does not know to specify a token value or forgets, we default to GH_TOKEN when I have specified remote.type = 'gitlab'.

How I tested

Build a paramaterized unit test with all the expected valid & invalid tests.

How to verify

git fetch origin pull/774/head:pr-774

# checkout the PR as a detached HEAD state with only the unit tests
git checkout pr-774~1 --detach

# execute tests, 2 should fail (non-github)
pytest tests/unit/semantic_release/cli/test_config.py

# update to the HEAD of the PR (with fixes)
git merge --ff-only pr-774

# run pytest again to see success
pytest tests/unit/semantic_release/cli/test_config.py

@codejedi365 codejedi365 force-pushed the refactor/dynamic-per-vcs-token-defaults branch from b8dde5b to abbce79 Compare December 17, 2023 23:23
@codejedi365 codejedi365 changed the title Refactor/dynamic per vcs token defaults refactor(config): enable default environment token per hvcs Dec 18, 2023
Copy link
Contributor

@bernardcooke53 bernardcooke53 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks solid, just a couple of minor comments - thanks @codejedi365 🚀

semantic_release/cli/config.py Outdated Show resolved Hide resolved
semantic_release/cli/config.py Show resolved Hide resolved
tests/unit/semantic_release/cli/test_config.py Outdated Show resolved Hide resolved
@codejedi365 codejedi365 force-pushed the refactor/dynamic-per-vcs-token-defaults branch 3 times, most recently from 48a06b1 to 8b8fe38 Compare December 20, 2023 02:51
@codejedi365 codejedi365 force-pushed the refactor/dynamic-per-vcs-token-defaults branch from 8b8fe38 to bb434f9 Compare December 20, 2023 02:53
Copy link
Contributor

@bernardcooke53 bernardcooke53 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @codejedi365! 🚀

@bernardcooke53 bernardcooke53 merged commit 26528eb into python-semantic-release:master Dec 22, 2023
8 checks passed
@codejedi365 codejedi365 deleted the refactor/dynamic-per-vcs-token-defaults branch December 22, 2023 13:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants